Add the prefixes to the cmake call in etc/CodeCoverage.sh for Coverity#9476
Conversation
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds logic to the Coverity script to include additional options in the cmake call by reading them from a file. While the intent is correct, there's a shell quoting issue in the implementation that would cause it to fail. My review provides a correction for this issue.
| if [[ -f "/etc/openroad_deps_prefixes.txt" ]]; then | ||
| cmakeOptions="$(cat "/etc/openroad_deps_prefixes.txt")" | ||
| fi | ||
| cmake "${cmakeOptions}" -B build . |
There was a problem hiding this comment.
The cmakeOptions variable is double-quoted in the cmake call. This will cause all the options read from the file to be passed as a single argument to cmake, which is incorrect. The variable should be expanded without quotes to allow the shell to perform word splitting, so that each option is passed as a separate argument.
Note that this approach can be fragile if any of the options or their values contain spaces. A more robust solution for that case would involve reading the options into a bash array.
| cmake "${cmakeOptions}" -B build . | |
| cmake ${cmakeOptions} -B build . |
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
No description provided.