Skip to content

Coverity fixes#4922

Merged
ethomson merged 6 commits into
libgit2:masterfrom
tiennou:fix/coverity-cids
Jan 8, 2019
Merged

Coverity fixes#4922
ethomson merged 6 commits into
libgit2:masterfrom
tiennou:fix/coverity-cids

Conversation

@tiennou

@tiennou tiennou commented Jan 4, 2019

Copy link
Copy Markdown
Contributor

Again, this fixes some of the latest Coverity reports.

Do note that the last commit attempts to make Coverity realize that clar's assertions are not dependent on DEBUG/NDEBUG, but it's (almost) a complete shot in the dark.

Ref: https://community.synopsys.com/s/question/0D534000046Yuzb/suppressing-assertsideeffect-for-functions-that-allow-for-sideeffects

Previously, an error in `git_config_next` would be mistaken as a
successful load, because the previous call would have succeeded.
Coverity saw the subsequent check for a completed iteration as dead, so
let's make it useful again.

CID 1391374
CID 1398597, 1398598
Coverity considers that anything that looks like assert() behaves like
it (ie. side-effects would be skipped on a NDEBUG build). As we have a
bunch of those in the test suite (128), this would ensure Coverity isn't
confused.
@ethomson

ethomson commented Jan 8, 2019

Copy link
Copy Markdown
Member

Thanks for the fixes. Be interested to see if the clar modeling works! 🤞

@ethomson ethomson merged commit 19bca3d into libgit2:master Jan 8, 2019
@tiennou tiennou deleted the fix/coverity-cids branch January 8, 2019 19:19
@tiennou

tiennou commented Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

It looks like the nightly Coverity run failed, though the logs aren't explicit about what. @ethomson Can you tell easily if it's an "infrastructure" fluke, or should I requeue the build ?

@tiennou

tiennou commented Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

Looks like Coverity is MIA : https://community.synopsys.com/s/article/Coverity-Scan-Update

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.

3 participants