From 6b77428be1591d1d858984bc449ea933e206390c Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Sun, 12 Apr 2026 23:34:12 +0200 Subject: [PATCH 1/9] Fix #1473: warn when fclose() is used as a while loop condition Using fclose() as a while loop condition closes the file on every iteration and operates on an already-closed file handle from the second iteration onward. Signed-off-by: Francois Berder --- lib/checkio.cpp | 13 +++++++++++++ test/testio.cpp | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 8b3c835caf3..c6d25899522 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -245,6 +245,19 @@ void CheckIO::checkFileUsage() } else if (tok->str() == "fclose") { fileTok = tok->tokAt(2); operation = Filepointer::Operation::CLOSE; + + // #1473 Check if fclose is in a while loop condition + const Token* tmp = tok->astParent(); + const Token* loopTok = nullptr; + while (tmp) { + if (Token::simpleMatch(tmp->previous(), "while (")) { + loopTok = tmp->previous(); + break; + } + tmp = tmp->astParent(); + } + if (loopTok) + useClosedFileError(tok); } else if (whitelist.find(tok->str()) != whitelist.end()) { fileTok = tok->tokAt(2); if ((tok->str() == "ungetc" || tok->str() == "ungetwc") && fileTok) diff --git a/test/testio.cpp b/test/testio.cpp index 02180f12376..fc502d90d02 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -544,7 +544,7 @@ class TestIO : public TestFixture { " FILE *a = fopen(\"aa\", \"r\");\n" " while (fclose(a)) {}\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:3:5]: (error) Used file that is not opened. [useClosedFile]\n", "", errout_str()); + ASSERT_EQUALS("[test.cpp:3:12]: (error) Used file that is not opened. [useClosedFile]\n", errout_str()); // #6823 check("void foo() {\n" From 440bef26c9ceabe936d97c205a0fc0ee7cb21ca4 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Mon, 27 Apr 2026 21:49:57 +0200 Subject: [PATCH 2/9] fixup! Fix #1473: warn when fclose() is used as a while loop condition --- lib/checkio.cpp | 26 +++++++++++++++++--------- test/testio.cpp | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index c6d25899522..7dc525dc1ae 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -247,17 +247,25 @@ void CheckIO::checkFileUsage() operation = Filepointer::Operation::CLOSE; // #1473 Check if fclose is in a while loop condition - const Token* tmp = tok->astParent(); - const Token* loopTok = nullptr; - while (tmp) { - if (Token::simpleMatch(tmp->previous(), "while (")) { - loopTok = tmp->previous(); - break; + if (fileTok) { + const Token* tmp = tok->astParent(); + const Token* loopTok = nullptr; + while (tmp) { + if (Token::simpleMatch(tmp->previous(), "while (")) { + loopTok = tmp->previous(); + break; + } + tmp = tmp->astParent(); + } + if (loopTok) { + const Token* bodyStart = loopTok->linkAt(1)->next(); + const Token* bodyEnd = bodyStart->link(); + + // Do not trigger a warning if the loop always exits or if the file is opened again in the loop. + if (!isReturnScope(bodyEnd, mSettings->library) && Token::findmatch(bodyStart, "%var% = fopen|freopen", bodyEnd, fileTok->varId()) == nullptr) + useClosedFileError(tok); } - tmp = tmp->astParent(); } - if (loopTok) - useClosedFileError(tok); } else if (whitelist.find(tok->str()) != whitelist.end()) { fileTok = tok->tokAt(2); if ((tok->str() == "ungetc" || tok->str() == "ungetwc") && fileTok) diff --git a/test/testio.cpp b/test/testio.cpp index fc502d90d02..5c137dec5dc 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -546,6 +546,22 @@ class TestIO : public TestFixture { "}"); ASSERT_EQUALS("[test.cpp:3:12]: (error) Used file that is not opened. [useClosedFile]\n", errout_str()); + check("void foo() {\n" + " FILE *a = fopen(\"aa\", \"r\");\n" + " while (fclose(a)) {\n" + " break;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout_str()); + + check("void foo() {\n" + " FILE *a = fopen(\"aa\", \"r\");\n" + " while (fclose(a)) {\n" + " a = fopen(\"aa\", \"r\");\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout_str()); + // #6823 check("void foo() {\n" " FILE f[2];\n" From 41fa0743d1028fb45f5fad1c222201a5ab639d1e Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Mon, 27 Apr 2026 22:33:56 +0200 Subject: [PATCH 3/9] fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- lib/checkio.cpp | 12 ++++++++++-- test/testio.cpp | 6 ++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 7dc525dc1ae..f32184909e5 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -258,8 +258,16 @@ void CheckIO::checkFileUsage() tmp = tmp->astParent(); } if (loopTok) { - const Token* bodyStart = loopTok->linkAt(1)->next(); - const Token* bodyEnd = bodyStart->link(); + const Token* bodyEnd = nullptr; + const Token* bodyStart = nullptr; + + if (Token::simpleMatch(loopTok->previous(), "}")) { // Handle do-while loops + bodyEnd = loopTok->previous(); + bodyStart = bodyEnd->link(); + } else { + bodyStart = loopTok->linkAt(1)->next(); + bodyEnd = bodyStart->link(); + } // Do not trigger a warning if the loop always exits or if the file is opened again in the loop. if (!isReturnScope(bodyEnd, mSettings->library) && Token::findmatch(bodyStart, "%var% = fopen|freopen", bodyEnd, fileTok->varId()) == nullptr) diff --git a/test/testio.cpp b/test/testio.cpp index 5c137dec5dc..fd14bd77d2c 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -562,6 +562,12 @@ class TestIO : public TestFixture { "}"); ASSERT_EQUALS("", errout_str()); + check("void foo() {\n" + " FILE *a = fopen(\"aa\", \"r\");\n" + " do {} while (fclose(a));\n" + "}"); + ASSERT_EQUALS("[test.cpp:3:18]: (error) Used file that is not opened. [useClosedFile]\n", errout_str()); + // #6823 check("void foo() {\n" " FILE f[2];\n" From e729474f952d631319ce9951c965a3b57738b930 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Tue, 28 Apr 2026 10:31:03 +0200 Subject: [PATCH 4/9] fixup! fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- lib/checkio.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index f32184909e5..160fb809476 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -247,7 +247,7 @@ void CheckIO::checkFileUsage() operation = Filepointer::Operation::CLOSE; // #1473 Check if fclose is in a while loop condition - if (fileTok) { + if (fileTok && fileTok->isVariable()) { const Token* tmp = tok->astParent(); const Token* loopTok = nullptr; while (tmp) { @@ -261,7 +261,7 @@ void CheckIO::checkFileUsage() const Token* bodyEnd = nullptr; const Token* bodyStart = nullptr; - if (Token::simpleMatch(loopTok->previous(), "}")) { // Handle do-while loops + if (Token::simpleMatch(loopTok->previous(), "}") && Token::simpleMatch(loopTok->previous()->link()->previous(), "do")) { // Handle do-while loops bodyEnd = loopTok->previous(); bodyStart = bodyEnd->link(); } else { @@ -270,7 +270,7 @@ void CheckIO::checkFileUsage() } // Do not trigger a warning if the loop always exits or if the file is opened again in the loop. - if (!isReturnScope(bodyEnd, mSettings->library) && Token::findmatch(bodyStart, "%var% = fopen|freopen", bodyEnd, fileTok->varId()) == nullptr) + if (!isReturnScope(bodyEnd, mSettings->library) && Token::findmatch(bodyStart, "%var% =", bodyEnd, fileTok->varId()) == nullptr) useClosedFileError(tok); } } From c60aa2ce562c715cfdc0ccd8aef70e1a1521b33a Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Wed, 29 Apr 2026 12:12:10 +0200 Subject: [PATCH 5/9] fixup! fixup! fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- lib/checkio.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 160fb809476..c6d3e8da93b 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -261,7 +261,7 @@ void CheckIO::checkFileUsage() const Token* bodyEnd = nullptr; const Token* bodyStart = nullptr; - if (Token::simpleMatch(loopTok->previous(), "}") && Token::simpleMatch(loopTok->previous()->link()->previous(), "do")) { // Handle do-while loops + if (Token::simpleMatch(loopTok->previous(), "}") && Token::simpleMatch(loopTok->linkAt(-1)->previous(), "do")) { // Handle do-while loops bodyEnd = loopTok->previous(); bodyStart = bodyEnd->link(); } else { From c4cbabc1464db13d9e74e55a3b3a32b8f2077e05 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Sun, 3 May 2026 21:47:29 +0200 Subject: [PATCH 6/9] fixup! fixup! fixup! fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- lib/checkio.cpp | 27 +++++++++++++++------------ lib/checkio.h | 1 + test/testio.cpp | 4 ++-- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index c6d3e8da93b..865e9ef64f2 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -248,20 +248,13 @@ void CheckIO::checkFileUsage() // #1473 Check if fclose is in a while loop condition if (fileTok && fileTok->isVariable()) { - const Token* tmp = tok->astParent(); - const Token* loopTok = nullptr; - while (tmp) { - if (Token::simpleMatch(tmp->previous(), "while (")) { - loopTok = tmp->previous(); - break; - } - tmp = tmp->astParent(); - } - if (loopTok) { + const Token* loopTok = tok->astTop()->previous(); + + if (loopTok && loopTok->str() == "while") { const Token* bodyEnd = nullptr; const Token* bodyStart = nullptr; - if (Token::simpleMatch(loopTok->previous(), "}") && Token::simpleMatch(loopTok->linkAt(-1)->previous(), "do")) { // Handle do-while loops + if (Token::simpleMatch(loopTok->previous(), "}") && loopTok->previous()->scope()->type == ScopeType::eDo) { // Handle do-while loops bodyEnd = loopTok->previous(); bodyStart = bodyEnd->link(); } else { @@ -271,7 +264,7 @@ void CheckIO::checkFileUsage() // Do not trigger a warning if the loop always exits or if the file is opened again in the loop. if (!isReturnScope(bodyEnd, mSettings->library) && Token::findmatch(bodyStart, "%var% =", bodyEnd, fileTok->varId()) == nullptr) - useClosedFileError(tok); + fcloseInLoopConditionError(tok, fileTok->str()); } } } else if (whitelist.find(tok->str()) != whitelist.end()) { @@ -421,6 +414,15 @@ void CheckIO::useClosedFileError(const Token *tok) "useClosedFile", "Used file that is not opened.", CWE910, Certainty::normal); } +void CheckIO::fcloseInLoopConditionError(const Token *tok, const std::string &varname) +{ + reportError(tok, Severity::warning, + "fcloseInLoopCondition", + "fclose() should not be used as a loop condition.\n" + "fclose() closes '" + varname + "' each time it is evaluated. On success the loop body might never execute, on failure fclose() might be called again on the already-closed file handle.", + CWE910, Certainty::normal); +} + void CheckIO::seekOnAppendedFileError(const Token *tok) { reportError(tok, Severity::warning, @@ -2072,6 +2074,7 @@ void CheckIO::getErrorMessages(ErrorLogger *errorLogger, const Settings *setting c.readWriteOnlyFileError(nullptr); c.writeReadOnlyFileError(nullptr); c.useClosedFileError(nullptr); + c.fcloseInLoopConditionError(nullptr, "fp"); c.seekOnAppendedFileError(nullptr); c.incompatibleFileOpenError(nullptr, "tmp"); c.invalidScanfError(nullptr); diff --git a/lib/checkio.h b/lib/checkio.h index e37a942770b..b3c86da3577 100644 --- a/lib/checkio.h +++ b/lib/checkio.h @@ -105,6 +105,7 @@ class CPPCHECKLIB CheckIO : public Check { void readWriteOnlyFileError(const Token *tok); void writeReadOnlyFileError(const Token *tok); void useClosedFileError(const Token *tok); + void fcloseInLoopConditionError(const Token *tok, const std::string &varname); void seekOnAppendedFileError(const Token *tok); void incompatibleFileOpenError(const Token *tok, const std::string &filename); void invalidScanfError(const Token *tok); diff --git a/test/testio.cpp b/test/testio.cpp index fd14bd77d2c..e7202a596f5 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -544,7 +544,7 @@ class TestIO : public TestFixture { " FILE *a = fopen(\"aa\", \"r\");\n" " while (fclose(a)) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3:12]: (error) Used file that is not opened. [useClosedFile]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:12]: (warning) fclose() should not be used as a loop condition. [fcloseInLoopCondition]\n", errout_str()); check("void foo() {\n" " FILE *a = fopen(\"aa\", \"r\");\n" @@ -566,7 +566,7 @@ class TestIO : public TestFixture { " FILE *a = fopen(\"aa\", \"r\");\n" " do {} while (fclose(a));\n" "}"); - ASSERT_EQUALS("[test.cpp:3:18]: (error) Used file that is not opened. [useClosedFile]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:18]: (warning) fclose() should not be used as a loop condition. [fcloseInLoopCondition]\n", errout_str()); // #6823 check("void foo() {\n" From d1d6247bdce6dbe465ff6863acf72f0633db1f05 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Mon, 11 May 2026 22:09:18 +0200 Subject: [PATCH 7/9] fixup! fixup! fixup! fixup! fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- lib/checkio.cpp | 2 +- test/testio.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 865e9ef64f2..cbf6101eb94 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -418,7 +418,7 @@ void CheckIO::fcloseInLoopConditionError(const Token *tok, const std::string &va { reportError(tok, Severity::warning, "fcloseInLoopCondition", - "fclose() should not be used as a loop condition.\n" + "fclose() used as loop condition may skip loop body or double-close file handle.\n" "fclose() closes '" + varname + "' each time it is evaluated. On success the loop body might never execute, on failure fclose() might be called again on the already-closed file handle.", CWE910, Certainty::normal); } diff --git a/test/testio.cpp b/test/testio.cpp index e7202a596f5..2c110388b95 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -544,7 +544,7 @@ class TestIO : public TestFixture { " FILE *a = fopen(\"aa\", \"r\");\n" " while (fclose(a)) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3:12]: (warning) fclose() should not be used as a loop condition. [fcloseInLoopCondition]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:12]: (warning) fclose() used as loop condition may skip loop body or double-close file handle. [fcloseInLoopCondition]\n", errout_str()); check("void foo() {\n" " FILE *a = fopen(\"aa\", \"r\");\n" @@ -566,7 +566,7 @@ class TestIO : public TestFixture { " FILE *a = fopen(\"aa\", \"r\");\n" " do {} while (fclose(a));\n" "}"); - ASSERT_EQUALS("[test.cpp:3:18]: (warning) fclose() should not be used as a loop condition. [fcloseInLoopCondition]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:18]: (warning) fclose() used as loop condition may skip loop body or double-close file handle. [fcloseInLoopCondition]\n", errout_str()); // #6823 check("void foo() {\n" From ad50ea72df1800e0c9a1a220df52784977147b43 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Fri, 15 May 2026 18:44:41 +0200 Subject: [PATCH 8/9] fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- man/checkers/fcloseInLoopCondition.md | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 man/checkers/fcloseInLoopCondition.md diff --git a/man/checkers/fcloseInLoopCondition.md b/man/checkers/fcloseInLoopCondition.md new file mode 100644 index 00000000000..db7f0a584c6 --- /dev/null +++ b/man/checkers/fcloseInLoopCondition.md @@ -0,0 +1,40 @@ +# fcloseInLoopCondition + +**Message**: fclose() used as loop condition may skip loop body or double-close file handle.
+**Category**: Resource Management
+**Severity**: Warning
+**Language**: C and C++ + +## Description + +Using `fclose()` as a loop condition leads to two unwanted outcomes: + +- On **success**, the condition is false and the loop body never executes. The intent was likely to process the file inside the loop, but it is already closed. +- On **failure**, the condition is true and the loop body executes but `fclose()` is called again on the already-closed file handle on the next iteration, which is undefined behaviour. + +This pattern is almost always a misunderstanding of what `fclose()` returns, or confusion with a function that reads/processes data incrementally (like `fgets` or `fread`). Unlike those functions, `fclose()` is a one-shot teardown operation and has no meaningful retry or loop-until-done semantic. + +## How to fix + +Call `fclose()` outside the loop condition. If you need to check whether the close succeeded, store the return value and test it separately. + +Before: +```c +FILE *fp = fopen("data.txt", "r"); +while (fclose(fp)) { + /* process file */ +} +``` + +After: +```c +FILE *fp = fopen("data.txt", "r"); +/* process file */ +if (fclose(fp) != 0) { + /* handle close error */ +} +``` + +## Related checkers + +- `useClosedFile` - for using a file handle that has already been closed From 30ed3691fdce190ba2effd5d2373788ea9a3c415 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Sat, 16 May 2026 17:06:44 +0200 Subject: [PATCH 9/9] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fix #1473: warn when fclose() is used as a while loop condition --- releasenotes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/releasenotes.txt b/releasenotes.txt index ec8c6032da9..5d94da2ebc3 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -8,6 +8,7 @@ New checks: - MISRA C 2012 rule 10.3 now warns on assigning integer literals 0 and 1 to bool in C99 and later while preserving the existing C89 behavior. - funcArgNamesDifferentUnnamed warns on function declarations/definitions where a parameter in either location is unnamed - uninitMemberVarNoCtor warns on user-defined types where some but not all members requiring initialization have in-class initializers. +- fcloseInLoopCondition warns when fclose() is used as a while loop condition, which may skip the loop body or double-close the file handle. C/C++ support: -