Fix #958: warn when feof() is used as a while loop condition#8422
Fix #958: warn when feof() is used as a while loop condition#8422francois-berder wants to merge 5 commits intodanmar:mainfrom
Conversation
feof() only returns true after a read has already failed, causing the loop body to execute once more after the last successful read. Read errors also go undetected since feof() does not distinguish them from EOF. Signed-off-by: Francois Berder <fberder@outlook.fr>
|
How does the check handle |
lib/checkio.cpp
Outdated
| if (!cond) | ||
| continue; |
There was a problem hiding this comment.
this code is redundant. simpleMatch returns false if the token is null
| if (!cond) | |
| continue; |
lib/checkio.cpp
Outdated
| { | ||
| reportError(tok, Severity::warning, | ||
| "wrongfeofUsage", | ||
| "Using feof() as a loop condition may cause the last line to be processed twice.\n" |
There was a problem hiding this comment.
I want that you remove the word "may". Can you somehow check if it is actually processed twice.
if there is no bug we shouldn't warn. example:
char line[100];
fgets(line, sizeof(line), f);
while (!feof(f)) {
dostuff(line);
fgets(line, sizeof(line), f);
}
There was a problem hiding this comment.
I improved this check by looking for a file read call before the loop and within the loop. This should cover this case but I am aware that it will lead to some FP/FN (cause by break statements in the loop body).
test/testio.cpp
Outdated
|
|
||
| void testWrongfeofUsage() { // ticket #958 | ||
| check("void foo() {\n" | ||
| " FILE * fp = fopen(\"test.txt\", \"r\");\n" |
There was a problem hiding this comment.
You can pass fp as parameter. then the testcase can be 2 lines shorter (no open/close).
|
thank you for adding this checker! it's good work. small and simple. just try to make it more precise. |
…ile loop condition
…as a while loop condition
|



feof() only returns true after a read has already failed, causing the loop body to execute once more after the last successful read. Read errors also go undetected since feof() does not distinguish them from EOF.