Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix minor issue found during static code scan #740

Merged

Conversation

DavidZagury
Copy link
Contributor

@DavidZagury DavidZagury commented Jan 19, 2023

Fix issue of using uninitialized value on unit tests.

Fix using uninitialized value when calling Compare on unit tests.
Fix Overrunning array of 16 bytes at byte offset 16 by dereferencing pointer ipa.ip_addr.ipv6_addr + mid + 1 on this case we call memset with a pointer to outside the block. This shouldn't cause any issues due to the fact that we call it with size to be written 0, but still should not be done.
prsunny
prsunny previously approved these changes Mar 1, 2023
@keboliu
Copy link
Collaborator

keboliu commented Mar 27, 2023

@prsunny @qiluo-msft would you please merge this PR?

@@ -76,7 +76,8 @@ class IpPrefix
{
assert(mid >= 0 && mid < 16);
ipa.ip_addr.ipv6_addr[mid] = (uint8_t)(0xFF << (8 - bits));
memset(ipa.ip_addr.ipv6_addr + mid + 1, 0, 16 - mid - 1);
if (mid < 15)
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (mid < 15)

Wondering who is complaining? Could you show the error/warning message of the compiler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qiluo-msft I replied in an email I sent you

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error does not make sense, it indicates the limitation of scanner/linter you are using.
I suggest not to sacrifice code concision to workaround scanner’s issue. If you like, you can add a code comment to suppress scanner negative alarm.

@liat-grozovik
Copy link
Collaborator

@qiluo-msft any further comments or can we merge this one?

@DavidZagury DavidZagury changed the title Fix minor issues found during static code scan Fix minor issue found during static code scan Jul 18, 2023
@qiluo-msft
Copy link
Contributor

/easycla

@DavidZagury
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 740 in repo sonic-net/sonic-swss-common

@DavidZagury
Copy link
Contributor Author

/azpw run Azure.sonic-swss-common

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss-common

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 740 in repo sonic-net/sonic-swss-common

@qiluo-msft qiluo-msft merged commit dc5135d into sonic-net:master Aug 14, 2023
13 checks passed
SviatoslavBoichuk pushed a commit to SviatoslavBoichuk/sonic-swss-common that referenced this pull request Sep 7, 2023
Fix issue of using uninitialized value on unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants