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

Add cppcheck run to travis build matrix #701

Merged
merged 7 commits into from
Jun 17, 2016

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Jun 16, 2016

This PR adds a new cppcheck build to the Travis CI build matrix for flux-core.

For the cppcheck build, a trivial script in src/test/cppcheck.sh is run instead of the normal build. In my short amount of testing this seemed to find some obvious errors like memory leaks, etc. A couple of these (in tests only) were fixed in this branch in order to run with a "clean" cppcheck initially.

There are also a list of paths that cppcheck.sh ignores by default, including upstream packages like liblsd, libsophia, and liblsd, as well as the autogenerated python bindings etc. The full list can be viewed (and updated) in the simple cppcheck.sh script.

Fix leaks in zmsg-test lua unit test to quiet cppcheck warnings.
Fix FILE handle leaks in kap detected by cppcheck.
@grondo grondo added the review label Jun 16, 2016
@coveralls
Copy link

coveralls commented Jun 16, 2016

Coverage Status

Coverage decreased (-0.04%) to 75.146% when pulling f6c08e7 on grondo:cppcheck into 87495ac on flux-framework:master.

@SteVwonder
Copy link
Member

SteVwonder commented Jun 16, 2016

Looks like there is an error that cppcheck picked up, but the test (according to Travis) still passed.
Direct link. If the link doesn't work, its line 7634.

@grondo
Copy link
Contributor Author

grondo commented Jun 16, 2016

Thanks I'll figure out if cppcheck isn't returning nonzero or if the wrapper script or something else is suppressing the error

@grondo
Copy link
Contributor Author

grondo commented Jun 16, 2016

Ok, I guess cppcheck doesn't exit nonzero by default when there are errors (I misread the docs), let's see if --error-exitcode=1 fixes the problem by leaving in the msglist test error. If so I'll fix that error and rebase the PR.

@coveralls
Copy link

coveralls commented Jun 16, 2016

Coverage Status

Coverage decreased (-0.03%) to 75.157% when pulling 305d50f on grondo:cppcheck into 87495ac on flux-framework:master.

@grondo
Copy link
Contributor Author

grondo commented Jun 16, 2016

msglist.c error seems to be a false positive to me.
For now I'll just suppress the error with a comment.

In any event, cppcheck error now does cause failure of the travis build (thanks @SteVwonder!), and now we'll check that a clean build passes.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 75.2% when pulling 8fb8df6 on grondo:cppcheck into 87495ac on flux-framework:master.

Suppress false positive doubleFree detected by cppcheck for
src/common/libutil/test/msglist.c
For git checkouts specifically, include any sha1, configure options,
and make options in the cache name so that package is rebuild if
any one of these items change.
Add the upstream cppcheck pkg to the list of packages built for
travis with travis-dep-builder.
Add a simple script to drive cppcheck in a standard way from
top level flux-core srcdir.
@coveralls
Copy link

coveralls commented Jun 16, 2016

Coverage Status

Coverage decreased (-0.03%) to 75.154% when pulling d2297da on grondo:cppcheck into 87495ac on flux-framework:master.

@garlick
Copy link
Member

garlick commented Jun 17, 2016

This is great! Ready to merge?

@grondo
Copy link
Contributor Author

grondo commented Jun 17, 2016

Yeah, this simple PR's ready

@garlick garlick merged commit 9a65636 into flux-framework:master Jun 17, 2016
@garlick garlick removed the review label Jun 17, 2016
@grondo grondo deleted the cppcheck branch June 25, 2016 01:01
@garlick garlick mentioned this pull request Aug 2, 2016
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.

4 participants