Skip to content

Coverity fixes#18014

Closed
paulidale wants to merge 5 commits intoopenssl:masterfrom
paulidale:coverity-fixes
Closed

Coverity fixes#18014
paulidale wants to merge 5 commits intoopenssl:masterfrom
paulidale:coverity-fixes

Conversation

@paulidale
Copy link
Copy Markdown
Contributor

All are false positives due to the up_ref/free setup we use. Added comments to hopefully quiet Coverity.

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

This is a false positive resulting from confusion over up_ref/free.
Another false positive tagged as such
Another reference counting false positive, now negated.
These are all false positives result from Coverity not understanding our
up_ref and free pairing.
@paulidale paulidale added branch: master Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending branch: 3.0 Applies to openssl-3.0 branch labels Apr 1, 2022
@paulidale paulidale self-assigned this Apr 1, 2022
@t8m
Copy link
Copy Markdown
Member

t8m commented Apr 1, 2022

Again I would not bother putting this into 3.0 branch. Also do we have an agreement that we want these ugly comments in code?

@t8m t8m added the triaged: documentation The issue/pr deals with documentation (errors) label Apr 1, 2022
@paulidale paulidale removed the branch: 3.0 Applies to openssl-3.0 branch label Apr 1, 2022
@paulidale
Copy link
Copy Markdown
Contributor Author

paulidale commented Apr 1, 2022

I'll drop 3.0. I've false positived a lot of these "free" problems while dealing with Coverity. I'm sure some of these are returns.

@t-j-h
Copy link
Copy Markdown
Member

t-j-h commented Apr 1, 2022

Note: we can also log these with Synopsys as false positives we want to get fixed.

@openssl-machine
Copy link
Copy Markdown
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

@paulidale
Copy link
Copy Markdown
Contributor Author

Ping.

@paulidale
Copy link
Copy Markdown
Contributor Author

Fixed.

@paulidale
Copy link
Copy Markdown
Contributor Author

reping

Copy link
Copy Markdown
Member

@slontis slontis left a comment

Choose a reason for hiding this comment

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

I also wish to know if 'we want these ugly comments in code?'..
Is this an OTC decision?

@paulidale
Copy link
Copy Markdown
Contributor Author

We decided to allow some but not a lot.

@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 May 5, 2022
@paulidale
Copy link
Copy Markdown
Contributor Author

Merged, thanks for the reviews.

@paulidale paulidale closed this May 6, 2022
openssl-machine pushed a commit that referenced this pull request May 6, 2022
This is a false positive resulting from confusion over up_ref/free.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #18014)
openssl-machine pushed a commit that referenced this pull request May 6, 2022
Another false positive tagged as such

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #18014)
openssl-machine pushed a commit that referenced this pull request May 6, 2022
Another reference counting false positive, now negated.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #18014)
openssl-machine pushed a commit that referenced this pull request May 6, 2022
These are all false positives result from Coverity not understanding our
up_ref and free pairing.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #18014)
@paulidale paulidale deleted the coverity-fixes branch September 5, 2022 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch triaged: documentation The issue/pr deals with documentation (errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants