Skip to content

Dm 1189 cppcheck

Afonso Mukai requested to merge DM-1189_cppcheck_base into master

Created by: SkyToGround

Description of work

My initial intent was originally to split this pull request into two. However, seeing as the number of lines of code added is only 355 and that most of the changes are very simple, I decided to create only one pull request for now. Please tell me if you would like me to split the pull request.

This is what I have done:

  • Updated cppcheck to the latest version (1.86).
  • Fixed a lot of issues found by cppcheck.
  • Removed a bunch of dead code related to issues found by cppcheck.
  • Changed naming to follow the LLVM standard in code that I touched anyway.

What I have NOT done:

  • Fix issues that I consider false positives.
  • Convert loops into stl algorithm calls, as suggested by the latest version of cppcheck. I might do this at some later point though.
  • Remove dead code not pointed out by cppcheck.

We are now down to 26 issues (from 131) of which around half are false positives and the rest are suggestions to replace loops with stl algorithm calls.

NOTE The file src/schemas/hs00/WriterTyped.h contains (almost) the only changes that I would consider non-trivial. The issues found were several uses of shadow variables. Furthermore, the file has several long methods, no unit test coverage and (as far as I can determine) no system/integration tests. I will leave it to you @dominikwerder to determine if you need to run manual tests of this code.

Issues

This pull request relates to DM-1189 but does not close the issue as more pull requests are coming.

Acceptance Criteria

Examine the code and determine of it appears sensible and that it introduces no changes in functionality.

Unit Tests

No unit tests were added or removed. However, several were modified to fix issues found by cppcheck.

Other

N/A

Code Review (To be filled in by the reviewer only)

  • Is the code of an acceptable quality?
  • Do the changes function as described and is it robust?

Nominate for Group Code Review (Anyone can nominate it)

Indicate if you think the code should be reviewed in a Thursday code review session.

  • Recommend for group code review

Also, nominate it on the code_review Slack channel (does someone want to automate this?).

Merge request reports