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

CppCheck: delete cppcheck-addon-ctu-file-list after usage #6984

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1420,8 +1420,11 @@ void CppCheck::executeAddons(const std::vector<std::string>& files, const std::s
std::string fileList;

if (files.size() >= 2 || endsWith(files[0], ".ctu-info")) {
// TODO: can this conflict when using -j?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we hit this issue in #6696.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a bug with -j here.

If I run cppcheck with an addon and use the -j flag and cppcheck analyzes the same source file twice for any reason (could be because its compiled twice with different flags) then it dies with:

error: Bailing out from analysis: Checking file failed: Failed to execute addon 'misra' - exitcode is 1 [internalError]

--verbose shows output from the plugin like this that makes it seem like one of the xml artifact files is malformed
https://gist.github.com/andymacg/69ff6f79e01bee2c75afdb7f674762cd
However when I manually inspected the xml file, it parsed OK, so I think its a race condition where two threads overwrite the same file.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a fix with #7082. But I am not sure how this can be tested effectively.

A unit test that ensures that the PID is part of the file list name? Or do you have any other suggestion?

fileList = Path::getPathFromFilename(files[0]) + FILELIST;
std::ofstream fout(fileList);
filesDeleter.addFile(fileList);
// TODO: check if file could be created
for (const std::string& f: files)
fout << f << std::endl;
}
Expand Down
Loading