Page MenuHome

[CPPCHECK] Fix parsing of shell-quoted paths
ClosedPublic

Authored by Loren Osborn (linux_dr) on Aug 28 2022, 6:25 PM.

Details

Summary

Description of the problem that is addressed in the patch

In some circumstances (like on "my machine") make check_cppcheck (which parses the --dry-run output of make encounters path names that are single-quoted. This causes the path to be misinterpreted and fail validation.

Description of the proposed solution, and a motivation as to why this is the best solution

If the command line is split according to shell rules, this would both unquote any quoted paths, and handle path names with spaces. (My $HOME does not contain spaces, so I haven't tested this case.)

List of alternative solutions, and a motivation as to why these are undesirable

We could instead hunt down the source of the path name quoting, but I think proper parsing of the command line arguments is more robust.

Limitations of the proposed solution

As implemented, this patch requires the python package shlex. I'm unsure if shlex is ubiquitous enough to be relied on. (I'm pretty new to python.)

Mock-up of the proposed user interface and a description of how users are going to interact with the new feature

make check_cppcheck works as intended, instead of failing with an error.

Diff Detail

Repository
rB Blender

Event Timeline

Loren Osborn (linux_dr) requested review of this revision.Aug 28 2022, 6:25 PM
Loren Osborn (linux_dr) created this revision.

It looks mostly like your code I modified.

Loren Osborn (linux_dr) edited the summary of this revision. (Show Details)Aug 28 2022, 6:46 PM

forgot to add cppcheck output file to .gitignore

Thanks for the patch, the shlex.split change can go in.

As for other changes, I'm not so happy with these analyzerinfo / snalyzerinfo files being written into the source directory.
Even if we ignore them, it means switching branches may fail (failure to remove non-empty directories). So I'd like to check on supporting an out-of-source path for these files.

As for /check_cppcheck.txt, it might be better to ignore /*.log and the make check_* targets can all write to *.log files, so we don't have to list every checkers file-name in .gitignore.

As for other changes, I'm not so happy with these analyzerinfo / snalyzerinfo files being written into the source directory.
Even if we ignore them, it means switching branches may fail (failure to remove non-empty directories). So I'd like to check on supporting an out-of-source path for these files.

As for /check_cppcheck.txt, it might be better to ignore /*.log and the make check_* targets can all write to *.log files, so we don't have to list every checkers file-name in .gitignore.

As we discussed via DM, my inclusion of these files in the .gitignore was not an endorsement of where these files are written. The intent is to prevent generated output files like these from being accidentally checked into the source tree. I wholeheartedly approve of any efforts to move them to the build folder; though adding this functionality myself may be a bit beyond my comfort level with Python, currently.

Accept shlex change, prefer to leave gitignore as-is.

Committed rB3baca3171978f99498d0aeba6fedd9f02961ca07.

This revision is now accepted and ready to land.Sep 9 2022, 9:19 AM