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

Coverity 2024 new hotness #17865

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

donaldsharp
Copy link
Member

see individual commits, fixing some coverity issues

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this will get rid of the SA errors. However, is there a reason you just didn't change the OSPF_API_MAX_MSG_SIZE to UINT16_MAX? The struct msg length limits it to that anyway,

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't know that there was a OSPF_API_MAX_MSG_SIZE. Let me look at that .

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

LGTM

msg_new takes a uint16_t, the length passed
down variable is a unsigned int, thus 32 bit.
It's possible, but highly unlikely, that the
msglen could be greater than 16 bit.
Let's just add some checks to ensure that
this could not happen.

Signed-off-by: Donald Sharp <[email protected]>
Grab the count of streams in ibuf when it is protected
by a mutex.  Since this data is written to it in another
pthread.

Signed-off-by: Donald Sharp <[email protected]>
The dg_update_list access is controlled by the dg_mutex in all
other locations.  Let's just add a mutex usage around the initialization
of the dg_update_list even if it's part of the startup, just to keep
things consistent.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp donaldsharp force-pushed the coverity_2024_new_hotness branch from c891ba3 to 19af3f3 Compare January 17, 2025 15:17
@ton31337 ton31337 requested a review from aceelindem January 20, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants