Skip to content

Fix #15010: ErrorLogger: do not open and read from files unless necessary - #8826

Open
ludviggunne wants to merge 2 commits into
cppcheck-opensource:mainfrom
ludviggunne:15010-redundant-read
Open

Fix #15010: ErrorLogger: do not open and read from files unless necessary#8826
ludviggunne wants to merge 2 commits into
cppcheck-opensource:mainfrom
ludviggunne:15010-redundant-read

Conversation

@ludviggunne

@ludviggunne ludviggunne commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Not exactly sure how to test the number of times the file is read.
I was thinking of adding a test under test/scripts using strace, but I'm not sure how robust it would be.

@ludviggunne
ludviggunne marked this pull request as draft September 4, 2026 12:32
@ludviggunne

Copy link
Copy Markdown
Collaborator Author

Before this change:

$ strace --trace=openat --trace-path=zerodiv.c --follow-forks ./cppcheck --suppress=zerodiv zerodiv.c
Checking zerodiv.c ...
openat(AT_FDCWD, "zerodiv.c", O_RDONLY) = 3
openat(AT_FDCWD, "zerodiv.c", O_RDONLY) = 3
openat(AT_FDCWD, "zerodiv.c", O_RDONLY) = 3
openat(AT_FDCWD, "zerodiv.c", O_RDONLY) = 3
openat(AT_FDCWD, "zerodiv.c", O_RDONLY) = 3
+++ exited with 0 +++

After this change:

$ strace --trace=openat --trace-path=zerodiv.c --follow-forks ./cppcheck --suppress=zerodiv zerodiv.c
zerodiv.c"
Checking zerodiv.c ...
openat(AT_FDCWD, "zerodiv.c", O_RDONLY) = 3
+++ exited with 0 +++

@ludviggunne
ludviggunne marked this pull request as ready for review September 4, 2026 14:31
@danmar

danmar commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

I was thinking of adding a test under test/scripts using strace, but I'm not sure how robust it would be.

Imho, such a test in test/cli that uses strace would be great. I guess it can be added in the other_test.py. It could test with --suppress , --xml and --template=cppcheck1 .. each of those options should mean the file is only opened once.

so I guess it will only run in linux but that is OK.

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.

2 participants