-
-
Notifications
You must be signed in to change notification settings - Fork 387
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 for SecBug cpp/comparison-with-wider-type #416
Fix for SecBug cpp/comparison-with-wider-type #416
Conversation
1d866ef
to
aa3419b
Compare
Since concurrency can be set to only a |
In that case, maybe we should cast the size_t into |
I think I want to test the effect of adding a cast first, I don't want to hamper performance (even if very slightly) just to get a tool not to throw an incorrect error. |
True I had the same thoughts, and that's why my first proposal was to use size_t so that it doesn't effect performance at all, and the code analysis tools are also happy! |
Seems like the only difference between the 2 versions is a single What's your opinion @kingster? |
IMHO the size_t is a simpler solution which also avoids a cast in the loop at least (even though it doesn't affect) and is future proof even if the type of concurrency is changed in future. |
I would prefer it if there was a comment above the loop specifying why |
42ee395
to
e2e19b5
Compare
Done. Pushed the updated changes, please have a look. |
e2e19b5
to
d49390b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems alright, will merge soon.
d49390b
to
de16aab
Compare
The return type for vector.size() is
size_t
which is larger thanuint16_t
and hence an overflow can cause infinite loop.Fixes this security bug https://codeql.github.com/codeql-query-help/cpp/cpp-comparison-with-wider-type/