Skip to content

Add static code analysis workflow with Cppcheck#1161

Merged
ddennedy merged 13 commits intomasterfrom
work/cppcheck-ci
Oct 22, 2025
Merged

Add static code analysis workflow with Cppcheck#1161
ddennedy merged 13 commits intomasterfrom
work/cppcheck-ci

Conversation

@jlskuz
Copy link
Copy Markdown
Contributor

@jlskuz jlskuz commented Oct 19, 2025

This adds a GitHub Workflow to run Cppcheck static code analysis. Despite the name Cppcheck also supports not only C++ but also C.

The results are not only visible in the log but also exported as GitHub Annotations.

Yet the job fails because Cppcheck finds some issues. These have to be reviewed now and either fixed or set to be ignored (in case of false positives).

@bmatherly
Copy link
Copy Markdown
Member

Cool! Thanks for working on this. I see you marked this as draft. Do you plan to resolve more of the findings? If so, let me know if you need any help.

@jlskuz
Copy link
Copy Markdown
Contributor Author

jlskuz commented Oct 22, 2025

Sorry for the late reply, I caught a cold and hence was offline. Thanks for your help @ddennedy and the offer @bmatherly !

@jlskuz jlskuz marked this pull request as ready for review October 22, 2025 20:53
@ddennedy ddennedy merged commit fd5cc7e into master Oct 22, 2025
21 checks passed
@ddennedy ddennedy deleted the work/cppcheck-ci branch October 22, 2025 21:04
@ddennedy ddennedy added this to the v7.34.0 milestone Oct 22, 2025
@bmatherly
Copy link
Copy Markdown
Member

image

I was hoping there would be some time for discussion before this was merged. But I guess there is still time.
I'm excited about the bugs that can be found with this tool. Here are some comments:

  • Should we put the command line arguments in a .cppcheck-config file so they do not have to be duplicated in two places?
  • Do we want a cmake target?
  • Is there any way to avoid the in-code cppcheck-suppress comments? I really don't like those. If we want to keep them, should we add an explanation for why the supression is being allowed?

@ddennedy
Copy link
Copy Markdown
Member

ddennedy commented Oct 22, 2025

You can move some things to the config file. See https://cppcheck.sourceforge.io/reference-cfg-format.pdf
You can make a PR with your ideas. The command lines are a little different between the two because --template used for the GitHub annotations breaks my IDEs' ability to click lines in the output.
Why do you think cmake is more appropriate for this than makefile?
Not all suppressions need to be inline, but I prefer that for line-oriented since the separated method breaks too easily as the line numbers change.

@ddennedy
Copy link
Copy Markdown
Member

P.S. I am going to make a release in a couple of days, which is one reason I moved this along.

@bmatherly
Copy link
Copy Markdown
Member

Why do you think cmake is more appropriate for this than makefile?

I'm not saying it is better. Just more "complete". I think we want cmake to be a first-class-citizen. And I know many projects have moved to using it. If someone is using cmake, they will not "discover" the cppcheck analysis - which has now become a requirement for code submission.

You can make a PR with your ideas.

Will do. Thanks.

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