From ff22173d0843ef547ed0868b83ff016753b3e0ea Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Tue, 7 Apr 2026 18:56:01 +0200 Subject: [PATCH 1/3] Fix #958: warn when feof() is used 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. Signed-off-by: Francois Berder --- lib/checkio.cpp | 31 +++++++++++++++++++++++++++++++ lib/checkio.h | 4 ++++ test/testio.cpp | 14 ++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index ee1b4e36c73..05688e30c9b 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -481,6 +481,36 @@ void CheckIO::invalidScanfError(const Token *tok) CWE119, Certainty::normal); } +void CheckIO::checkWrongfeofUsage() +{ + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + + logChecker("CheckIO::checkWrongfeofUsage"); + + for (const Scope * scope : symbolDatabase->functionScopes) { + for (const Token *tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) { + if (Token::simpleMatch(tok, "while (")) { + const Token* cond = getCondTok(tok); + if (!cond) + continue; + if (Token::Match(cond, "! feof (")) { + wrongfeofUsage(cond); + } + } + } + } +} + +void CheckIO::wrongfeofUsage(const Token * tok) +{ + reportError(tok, Severity::warning, + "wrongfeofUsage", + "Using feof() as a loop condition may cause the last line to be processed twice.\n" + "feof() returns true only after a read has failed due to end-of-file, so the loop " + "body executes once more after the last successful read. Check the return value of " + "the read function instead (e.g. fgets, fread, fscanf)."); +} + //--------------------------------------------------------------------------- // printf("%u", "xyz"); // Wrong argument type // printf("%u%s", 1); // Too few arguments @@ -2031,6 +2061,7 @@ void CheckIO::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) checkIO.checkWrongPrintfScanfArguments(); checkIO.checkCoutCerrMisusage(); checkIO.checkFileUsage(); + checkIO.checkWrongfeofUsage(); checkIO.invalidScanf(); } diff --git a/lib/checkio.h b/lib/checkio.h index e37a942770b..e58854f3d57 100644 --- a/lib/checkio.h +++ b/lib/checkio.h @@ -64,6 +64,9 @@ class CPPCHECKLIB CheckIO : public Check { /** @brief scanf can crash if width specifiers are not used */ void invalidScanf(); + /** @brief %Check wrong usage of feof */ + void checkWrongfeofUsage(); + /** @brief %Checks type and number of arguments given to functions like printf or scanf*/ void checkWrongPrintfScanfArguments(); @@ -108,6 +111,7 @@ class CPPCHECKLIB CheckIO : public Check { void seekOnAppendedFileError(const Token *tok); void incompatibleFileOpenError(const Token *tok, const std::string &filename); void invalidScanfError(const Token *tok); + void wrongfeofUsage(const Token *tok); void wrongPrintfScanfArgumentsError(const Token* tok, const std::string &functionName, nonneg int numFormat, diff --git a/test/testio.cpp b/test/testio.cpp index b402f5c0aa1..483c11c3757 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -45,6 +45,7 @@ class TestIO : public TestFixture { TEST_CASE(seekOnAppendedFile); TEST_CASE(fflushOnInputStream); TEST_CASE(incompatibleFileOpen); + TEST_CASE(testWrongfeofUsage); // #958 TEST_CASE(testScanf1); // Scanf without field limiters TEST_CASE(testScanf2); @@ -743,6 +744,19 @@ class TestIO : public TestFixture { ASSERT_EQUALS("[test.cpp:3:16]: (warning) The file '\"tmp\"' is opened for read and write access at the same time on different streams [incompatibleFileOpen]\n", errout_str()); } + void testWrongfeofUsage() { // ticket #958 + check("void foo() {\n" + " FILE * fp = fopen(\"test.txt\", \"r\");\n" + " while (!feof(fp)) \n" + " {\n" + " char line[100];\n" + " fgets(line, sizeof(line), fp);\n" + " }\n" + " fclose(fp);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3:10]: (warning) Using feof() as a loop condition may cause the last line to be processed twice. [wrongfeofUsage]\n", errout_str()); + } + void testScanf1() { check("void foo() {\n" From 5ef23b1439144f2e91679f6f99574f4721f9b4d7 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Tue, 7 Apr 2026 23:47:41 +0200 Subject: [PATCH 2/3] fixup! Fix #958: warn when feof() is used as a while loop condition --- lib/checkio.cpp | 1 + test/testio.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 05688e30c9b..0b97ebd5ce2 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -2076,6 +2076,7 @@ void CheckIO::getErrorMessages(ErrorLogger *errorLogger, const Settings *setting c.useClosedFileError(nullptr); c.seekOnAppendedFileError(nullptr); c.incompatibleFileOpenError(nullptr, "tmp"); + c.wrongfeofUsage(nullptr); c.invalidScanfError(nullptr); c.wrongPrintfScanfArgumentsError(nullptr, "printf",3,2); c.invalidScanfArgTypeError_s(nullptr, 1, "s", nullptr); diff --git a/test/testio.cpp b/test/testio.cpp index 483c11c3757..528775c37b4 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -755,6 +755,17 @@ class TestIO : public TestFixture { " fclose(fp);\n" "}"); ASSERT_EQUALS("[test.cpp:3:10]: (warning) Using feof() as a loop condition may cause the last line to be processed twice. [wrongfeofUsage]\n", errout_str()); + + check("int foo() {\n" + " FILE * fp = fopen(\"test.txt\", \"r\");\n" + " char line[100];\n" + " while (fgets(line, sizeof(line), fp)) {}\n" + " if (!feof(fp))\n" + " return 1;\n" + " fclose(fp);\n" + " return 0;\n" + "}"); + ASSERT_EQUALS("", errout_str()); } From 368df3b45acbd91be9bb9d990c77bd1ae3d8d1c0 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Wed, 8 Apr 2026 12:30:07 +0200 Subject: [PATCH 3/3] fixup! fixup! Fix #958: warn when feof() 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 0b97ebd5ce2..b76294a4685 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -493,7 +493,7 @@ void CheckIO::checkWrongfeofUsage() const Token* cond = getCondTok(tok); if (!cond) continue; - if (Token::Match(cond, "! feof (")) { + if (Token::simpleMatch(cond, "! feof (")) { wrongfeofUsage(cond); } }