search

resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
495 stars 49 forks source link

ci: add cppcheck for static analysis #322

Closed N-R-K closed 1 year ago

N-R-K commented 1 year ago

cppcheck is a decent static analyzer that can catch some interesting issues that compiler warnings (and sometimes even other analyzers) don't catch.

currently cppcheck doesn't find anything, but that's because I had mostly fixed it's finding very early on when I started contributing to scrot.

I've had cppcheck, along with gcc -fanalyzer and clang-tidy running locally for quite some time now. it will be worthwhile to add it to the CI so that it's findings can be visible to other contributors as well.

in the future, we might want to enable -fanalyzer and clang-tidy as well, but clang-tidy currently produces 3 warnings, 2 of which I've investigated and found to be false-positives (haven't investigated the 3rd one).

to run clang-tidy locally:

$ clang-tidy --quiet --checks="-clang-analyzer-security.insecureAPI.*" \
       src/*.c -- -Wall -Wextra -Wpedantic -DDEBUG \
       $(pkg-config --cflags ./deps.pc)

(note, I've disabled the "insecureAPI" check because it is garbage. it flags every memcpy call as "insecure" and tells you to use the nonportable memcpy_s instead.)

gcc -fanalyzer is relatively new and not as mature as cppcheck or clang-tidy but it's getting better each version. currently it spits out a bunch of warnings, I haven't investigated them too thoroughly but they seem like false-positives.

to run gcc -fanalyzer locally:

$ gcc -o /dev/null src/*.c -std=c99 -Wall -Wextra -Wpedantic \
      -O3 -flto -fanalyzer $(pkg-config --cflags --libs ./deps.pc)

note: both gcc/clang-tidy accepts -Werror, which we probably want to enable if we put it in the CI.

guijan commented 1 year ago

Okay, Now I like that "decouple the source from autotools" commit :D. Linters are good.

N-R-K commented 1 year ago

Linters are good.

I plan on adding clang-tidy with a customized list of checks next. The list of checks used by nsxiv can serve as a good baseline.

Note to self: figure out if clang-tidy's default yaml config format is capable of handling multi-line list of checks or not (last time I tried, it couldn't. But maybe it's been fixed now).

Another note to self: the imPrintf function might make a good target for fuzzing. I'll probably set it up after I'm done with clang-tidy.