Fix #958: warn when feof() is used as a while loop condition

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::simpleMatch(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();
 }
 
@@ -2045,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);

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,

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,30 @@ 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());
+
+        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());
+    }
+
 
     void testScanf1() {
         check("void foo() {\n"