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 Coverity 1493746: constant expression result #17034

Closed

Conversation

paulidale
Copy link
Contributor

Turning an always false expression into a real test that doesn't do anything apart from appease the compiler.

  • documentation is added or updated
  • tests are added or updated

@paulidale paulidale added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Nov 14, 2021
@paulidale paulidale self-assigned this Nov 14, 2021
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Nov 14, 2021
@mattcaswell
Copy link
Member

Turning an always false expression into a real test that doesn't do anything apart from appease the compiler.

If it doesn't do anything...why do we need it?

@paulidale
Copy link
Contributor Author

It prevents a compiler warning. For compilers where sizeof(int) == sizeof(size_t), the test is real, legitimate and required.

Rather than ignoring the report, I figured putting a proper check in place would prevent all future reports and warnings from our current and some future compilers. I'm fine tagging this as a false positive that is to be ignored. It will almost certainly revisit us in the future.

@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Nov 15, 2021
@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Nov 15, 2021
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Nov 16, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Nov 16, 2021
@paulidale
Copy link
Contributor Author

Merged, thanks for the feedback.

@paulidale paulidale closed this Nov 16, 2021
@paulidale paulidale deleted the coverity-1493746-const-expr branch November 16, 2021 22:16
openssl-machine pushed a commit that referenced this pull request Nov 16, 2021
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #17034)
ramikhaldi pushed a commit to ramikhaldi/openssl that referenced this pull request Nov 21, 2021
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#17034)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch severity: fips change The pull request changes FIPS provider sources triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants