From 86e6bb430a9a8b8c6e170bfeb61ce8dbf053f9c6 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Mon, 17 Mar 2014 11:02:03 +0100 Subject: [PATCH] New check: Warning, if positioning operation (fseek) is performed on a file opened in "a" mode --- lib/checkio.cpp | 40 ++++++++++++++++++++++++++++------------ lib/checkio.h | 3 +++ test/testio.cpp | 24 +++++++++++++++++++++++- test/testnullpointer.cpp | 2 +- 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index c5bc3427a..11fddee21 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -72,7 +72,7 @@ void CheckIO::coutCerrMisusageError(const Token* tok, const std::string& streamN // fopen("","r"); fwrite(); <- write to read-only file (or vice versa) // fclose(); fread(); <- Use closed file //--------------------------------------------------------------------------- -enum OpenMode {CLOSED, READ_MODE, WRITE_MODE, RW_MODE, UNKNOWN}; +enum OpenMode { CLOSED, READ_MODE, WRITE_MODE, RW_MODE, UNKNOWN_OM }; static OpenMode getMode(const std::string& str) { if (str.find('+', 1) != std::string::npos) @@ -81,7 +81,7 @@ static OpenMode getMode(const std::string& str) return WRITE_MODE; else if (str.find('r') != std::string::npos) return READ_MODE; - return UNKNOWN; + return UNKNOWN_OM; } struct Filepointer { @@ -89,8 +89,10 @@ struct Filepointer { unsigned int mode_indent; enum Operation {NONE, UNIMPORTANT, READ, WRITE, POSITIONING, OPEN, CLOSE, UNKNOWN_OP} lastOperation; unsigned int op_indent; - Filepointer(OpenMode mode_ = UNKNOWN) - : mode(mode_), mode_indent(0), lastOperation(NONE), op_indent(0) { + enum AppendMode { UNKNOWN_AM, APPEND, APPEND_EX }; + AppendMode append_mode; + Filepointer(OpenMode mode_ = UNKNOWN_OM) + : mode(mode_), mode_indent(0), lastOperation(NONE), op_indent(0), append_mode(UNKNOWN_AM) { } }; @@ -113,11 +115,11 @@ void CheckIO::checkFileUsage() if (var->isLocal()) { if (var->nameToken()->strAt(1) == "(") // initialize by calling "ctor" - filepointers.insert(std::make_pair(var->declarationId(), Filepointer(UNKNOWN))); + filepointers.insert(std::make_pair(var->declarationId(), Filepointer(UNKNOWN_OM))); else filepointers.insert(std::make_pair(var->declarationId(), Filepointer(CLOSED))); } else { - filepointers.insert(std::make_pair(var->declarationId(), Filepointer(UNKNOWN))); + filepointers.insert(std::make_pair(var->declarationId(), Filepointer(UNKNOWN_OM))); // TODO: If all fopen calls we find open the file in the same type, we can set Filepointer::mode } } @@ -134,7 +136,7 @@ void CheckIO::checkFileUsage() for (std::map::iterator i = filepointers.begin(); i != filepointers.end(); ++i) { if (indent < i->second.mode_indent) { i->second.mode_indent = 0; - i->second.mode = UNKNOWN; + i->second.mode = UNKNOWN_OM; } if (indent < i->second.op_indent) { i->second.op_indent = 0; @@ -144,7 +146,7 @@ void CheckIO::checkFileUsage() } else if (tok->str() == "return" || tok->str() == "continue" || tok->str() == "break") { // Reset upon return, continue or break for (std::map::iterator i = filepointers.begin(); i != filepointers.end(); ++i) { i->second.mode_indent = 0; - i->second.mode = UNKNOWN; + i->second.mode = UNKNOWN_OM; i->second.op_indent = 0; i->second.lastOperation = Filepointer::UNKNOWN_OP; } @@ -153,7 +155,7 @@ void CheckIO::checkFileUsage() (windows ? (tok->str() != "_wfopen" && tok->str() != "_wfreopen") : true))) { std::map::iterator i = filepointers.find(tok->varId()); if (i != filepointers.end()) { - i->second.mode = UNKNOWN; + i->second.mode = UNKNOWN_OM; i->second.lastOperation = Filepointer::UNKNOWN_OP; } } else if (Token::Match(tok, "%var% (") && tok->previous() && (!tok->previous()->isName() || Token::Match(tok->previous(), "return|throw"))) { @@ -230,18 +232,26 @@ void CheckIO::checkFileUsage() continue; if (filepointers.find(fileTok->varId()) == filepointers.end()) { // function call indicates: Its a File - filepointers.insert(std::make_pair(fileTok->varId(), Filepointer(UNKNOWN))); + filepointers.insert(std::make_pair(fileTok->varId(), Filepointer(UNKNOWN_OM))); } Filepointer& f = filepointers[fileTok->varId()]; switch (operation) { case Filepointer::OPEN: f.mode = getMode(mode); + if (mode.find('a') != std::string::npos) { + if (f.mode == RW_MODE) + f.append_mode = Filepointer::APPEND_EX; + else + f.append_mode = Filepointer::APPEND; + } f.mode_indent = indent; break; case Filepointer::POSITIONING: if (f.mode == CLOSED) useClosedFileError(tok); + else if (f.append_mode == Filepointer::APPEND && _settings->isEnabled("warning")) + seekOnAppendedFileError(tok); break; case Filepointer::READ: if (f.mode == CLOSED) @@ -271,7 +281,7 @@ void CheckIO::checkFileUsage() useClosedFileError(tok); break; case Filepointer::UNKNOWN_OP: - f.mode = UNKNOWN; + f.mode = UNKNOWN_OM; f.mode_indent = 0; break; default: @@ -285,7 +295,7 @@ void CheckIO::checkFileUsage() } for (std::map::iterator i = filepointers.begin(); i != filepointers.end(); ++i) { i->second.op_indent = 0; - i->second.mode = UNKNOWN; + i->second.mode = UNKNOWN_OM; i->second.lastOperation = Filepointer::UNKNOWN_OP; } } @@ -322,6 +332,12 @@ void CheckIO::useClosedFileError(const Token *tok) "useClosedFile", "Used file that is not opened."); } +void CheckIO::seekOnAppendedFileError(const Token *tok) +{ + reportError(tok, Severity::warning, + "seekOnAppendedFile", "Repositioning operation performed on a file opened in append mode has no effect."); +} + //--------------------------------------------------------------------------- // scanf without field width limits can crash with huge input data diff --git a/lib/checkio.h b/lib/checkio.h index fbdd336e3..12fa4b2f0 100644 --- a/lib/checkio.h +++ b/lib/checkio.h @@ -100,6 +100,7 @@ private: void readWriteOnlyFileError(const Token *tok); void writeReadOnlyFileError(const Token *tok); void useClosedFileError(const Token *tok); + void seekOnAppendedFileError(const Token *tok); void invalidScanfError(const Token *tok, bool portability); void wrongPrintfScanfArgumentsError(const Token* tok, const std::string &function, @@ -130,6 +131,7 @@ private: c.readWriteOnlyFileError(0); c.writeReadOnlyFileError(0); c.useClosedFileError(0); + c.seekOnAppendedFileError(0); c.invalidScanfError(0, false); c.wrongPrintfScanfArgumentsError(0,"printf",3,2); c.invalidScanfArgTypeError_s(0, 1, "s", NULL); @@ -157,6 +159,7 @@ private: "* Use a file that has been closed\n" "* File input/output without positioning results in undefined behaviour\n" "* Read to a file that has only been opened for writing (or vice versa)\n" + "* Repositioning operation on a file opened in append mode\n" "* Using fflush() on an input stream\n" "* Invalid usage of output stream. For example: 'std::cout << std::cout;'\n" "* Wrong number of arguments given to 'printf' or 'scanf;'\n"; diff --git a/test/testio.cpp b/test/testio.cpp index 0298bb8de..7e364422a 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -37,6 +37,7 @@ private: TEST_CASE(wrongMode_complex); TEST_CASE(useClosedFile); TEST_CASE(fileIOwithoutPositioning); + TEST_CASE(seekOnAppendedFile); TEST_CASE(fflushOnInputStream); TEST_CASE(testScanf1); // Scanf without field limiters @@ -251,7 +252,8 @@ private: " rewind(f);\n" " fread(buffer, 5, 6, f);\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Read operation on a file that was opened only for writing.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (warning) Repositioning operation performed on a file opened in append mode has no effect.\n" + "[test.cpp:5]: (error) Read operation on a file that was opened only for writing.\n", errout.str()); check("void foo(FILE*& f) {\n" " f = fopen(name, \"a+\");\n" @@ -530,6 +532,26 @@ private: ASSERT_EQUALS("[test.cpp:4]: (error) Read and write operations without a call to a positioning function (fseek, fsetpos or rewind) or fflush in between result in undefined behaviour.\n", errout.str()); } + void seekOnAppendedFile() { + check("void foo() {\n" + " FILE* f = fopen(\"\", \"a+\");\n" + " fseek(f, 0, SEEK_SET);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " FILE* f = fopen(\"\", \"w\");\n" + " fseek(f, 0, SEEK_SET);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " FILE* f = fopen(\"\", \"a\");\n" + " fseek(f, 0, SEEK_SET);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Repositioning operation performed on a file opened in append mode has no effect.\n", errout.str()); + } + void fflushOnInputStream() { check("void foo()\n" "{\n" diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index f8fc4b075..a7a388921 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2417,7 +2417,7 @@ private: check("void f(char*p,char*q){ strcoll (p,q);if(!p||!q){}}"); ASSERT_EQUALS(errpq,errout.str()); - + check("void f(char*p,char*q){ strcat (p,q);if(!p||!q){}}"); ASSERT_EQUALS(errpq,errout.str());