Fix ticket #93 (Write xml results into error stream instead of results.xml file.) and also refactor the

code to use ErrorLogger::reportErr() for all errors, for both xml and plain text. And move xml formatting 
from Cppcheck to CppcheckExecutor.

man/cppcheck.1.xml

@@ -110,7 +110,7 @@ man(1), man(7), http://www.tldp.org/HOWTO/Man-Page/
       <arg choice="opt"><option>--style</option></arg>
       <arg choice="opt"><option>--unused-functions</option></arg>
       <arg choice="opt"><option>--verbose</option></arg>
-      <arg choice="opt"><option>--xml-results</option></arg>
+      <arg choice="opt"><option>--xml</option></arg>
       <arg choice="opt"><option>file or path</option></arg>
       <arg choice="plain"><option>...</option></arg>
     </cmdsynopsis>
@@ -187,9 +187,9 @@ files, this is not needed.</para>
         </listitem>
       </varlistentry>
       <varlistentry>
-        <term><option>--xml-results</option></term>
+        <term><option>--xml</option></term>
         <listitem>
-          <para>Write results in results.xml</para>
+          <para>Write results in xml to error stream</para>
         </listitem>
       </varlistentry>
     </variablelist>

src/checkdangerousfunctions.cpp

@@ -56,17 +56,17 @@ void CheckDangerousFunctionsClass::dangerousFunctions()
 {
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
-        if (Token::Match(tok, "mktemp ("))
+        if (Token::simpleMatch(tok, "mktemp ("))
         {
-            std::ostringstream ostr;
+            _errorLogger->dangerousFunctionmktemp(_tokenizer, tok);
-            ostr << _tokenizer->fileLine(tok) << ": Found 'mktemp'. You should use 'mkstemp' instead";
-            _errorLogger->reportErr(ostr.str());
         }
-        else if (Token::Match(tok, "gets|scanf ("))
+        else if (Token::simpleMatch(tok, "gets ("))
         {
-            std::ostringstream ostr;
+            _errorLogger->dangerousFunctiongets(_tokenizer, tok);
-            ostr << _tokenizer->fileLine(tok) << ": Found '" << tok->str() << "'. You should use 'fgets' instead";
+        }
-            _errorLogger->reportErr(ostr.str());
+        else if (Token::simpleMatch(tok, "scanf ("))
+        {
+            _errorLogger->dangerousFunctionscanf(_tokenizer, tok);
         }
     }
 }

src/checkheaders.cpp

@@ -60,7 +60,13 @@ void CheckHeaders::WarningHeaderWithImplementation()
         {
             std::ostringstream ostr;
             ostr << _tokenizer->fileLine(tok) << ": Found implementation in header";
-            _errorLogger->reportErr(ostr.str());
+
+            // TODO, this check is currently not used, but if it is some day
+            // it should give correct id and severity by calling proper function
+            // from errorLogger. It should not call reportErr directly.
+            std::list<FileLocation> empty;
+            empty.push_back(FileLocation());
+            _errorLogger->reportErr(empty, "id", "severity", ostr.str());
 
             // Goto next file..
             unsigned int fileindex = tok->fileIndex();
@@ -248,7 +254,13 @@ void CheckHeaders::WarningIncludeHeader()
             ostr << _tokenizer->fileLine(includetok) << ": The included header '" << includefile << "' is not needed";
             if (NeedDeclaration)
                 ostr << " (but a forward declaration is needed)";
-            _errorLogger->reportErr(ostr.str());
+
+            // TODO, this check is currently not used, but if it is some day
+            // it should give correct id and severity by calling proper function
+            // from errorLogger. It should not call reportErr directly.
+            std::list<FileLocation> empty;
+            empty.push_back(FileLocation());
+            _errorLogger->reportErr(empty, "id", "severity", ostr.str()); // TODO
         }
     }
 }

src/cppcheck.cpp

@@ -186,7 +186,7 @@ std::string CppCheck::parseFromArgs(int argc, const char* const argv[])
         "    -s, --style          Check coding style\n"
         "    --unused-functions   Check if there are unused functions\n"
         "    -v, --verbose        More detailed error reports\n"
-        "    --xml                Write results in results.xml\n"
+        "    --xml                Write results in xml to error stream.\n"
         "\n"
         "Example usage:\n"
         "  # Recursively check the current folder. Print the progress on the screen and\n"
@@ -284,15 +284,7 @@ unsigned int CppCheck::check()
         _checkFunctionUsage.check();
     }
 
-    if (_settings._xml)
+
-    {
-        std::ofstream fxml("result.xml");
-        fxml << "<?xml version=\"1.0\"?>\n";
-        fxml << "<results>\n";
-        for (std::list<std::string>::const_iterator it = _xmllist.begin(); it != _xmllist.end(); ++it)
-            fxml << "  " << *it << "\n";
-        fxml << "</results>";
-    }
 
     unsigned int result = static_cast<unsigned int>(_errorList.size());
     _errorList.clear();
@@ -416,10 +408,20 @@ void CppCheck::checkFile(const std::string &code, const char FileName[])
     if (ErrorLogger::strPlusChar())
         checkOther.strPlusChar();
 }
+
+Settings CppCheck::settings() const
+{
+    return _settings;
+}
+
 //---------------------------------------------------------------------------
 
-void CppCheck::reportErr(const std::string &errmsg)
+void CppCheck::reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg)
 {
+    std::ostringstream text;
+    text << ErrorLogger::callStackToString(callStack) << ": (" << severity << ") " << msg;
+    std::string errmsg = text.str();
+
     // Alert only about unique errors
     if (std::find(_errorList.begin(), _errorList.end(), errmsg) != _errorList.end())
         return;
@@ -431,8 +433,7 @@ void CppCheck::reportErr(const std::string &errmsg)
         errmsg2 += "\n    Defines=\'" + cfg + "\'\n";
     }
 
-
+    _errorLogger->reportErr(callStack, id, severity, msg);
-    _errorLogger->reportErr(errmsg2);
 
     _errout << errmsg2 << std::endl;
 }
@@ -442,17 +443,3 @@ void CppCheck::reportOut(const std::string & /*outmsg*/)
     // This is currently never called. It is here just to comply with
     // the interface.
 }
-
-void CppCheck::reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg)
-{
-    std::ostringstream xml;
-    xml << "<error";
-    xml << " file=\"" << file << "\"";
-    xml << " line=\"" << line << "\"";
-    xml << " id=\"" << id << "\"";
-    xml << " severity=\"" << severity << "\"";
-    xml << " msg=\"" << msg << "\"";
-    xml << "/>";
-
-    _xmllist.push_back(xml.str());
-}

src/cppcheck.h

@@ -63,6 +63,12 @@ public:
      */
     void settings(const Settings &settings);
 
+    /**
+     * Get copy of current settings.
+     * @return a copy of current settings
+     */
+    Settings settings() const;
+
     /**
      * Add new file to be checked.
      *
@@ -104,7 +110,7 @@ private:
      * "[filepath:line number] Message", e.g.
      * "[main.cpp:4] Uninitialized member variable"
      */
-    virtual void reportErr(const std::string &errmsg);
+    virtual void reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg);
 
     /**
      * Information about progress is directed here.
@@ -113,9 +119,6 @@ private:
      */
     virtual void reportOut(const std::string &outmsg);
 
-    /** xml output of errors */
-    virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg);
-
     std::list<std::string> _errorList;
     std::ostringstream _errout;
     Settings _settings;

src/cppcheckexecutor.cpp

@@ -24,7 +24,7 @@
 
 CppCheckExecutor::CppCheckExecutor()
 {
-    //ctor
+    _useXML = false;
 }
 
 CppCheckExecutor::~CppCheckExecutor()
@@ -38,7 +38,20 @@ unsigned int CppCheckExecutor::check(int argc, const char* const argv[])
     std::string result = cppCheck.parseFromArgs(argc, argv);
     if (result.length() == 0)
     {
-        return cppCheck.check();
+        if (cppCheck.settings()._xml)
+        {
+            _useXML = true;
+            reportErr("<?xml version=\"1.0\"?>");
+            reportErr("<results>");
+        }
+
+        unsigned int returnValue = cppCheck.check();
+        if (_useXML)
+        {
+            reportErr("</results>");
+        }
+
+        return returnValue;
     }
     else
     {
@@ -57,7 +70,25 @@ void CppCheckExecutor::reportOut(const std::string &outmsg)
     std::cout << outmsg << std::endl;
 }
 
-void CppCheckExecutor::reportXml(const std::string & /*file*/, const std::string & /*line*/, const std::string & /*id*/, const std::string & /*severity*/, const std::string & /*msg*/)
+void CppCheckExecutor::reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg)
 {
-    // never used
+    if (_useXML)
+    {
+        std::ostringstream xml;
+        xml << "<error";
+        xml << " file=\"" << callStack.back().file << "\"";
+        xml << " line=\"" << callStack.back().line << "\"";
+        xml << " id=\"" << id << "\"";
+        xml << " severity=\"" << severity << "\"";
+        xml << " msg=\"" << msg << "\"";
+        xml << "/>";
+        reportErr(xml.str());
+    }
+    else
+    {
+        std::ostringstream text;
+
+        text << ErrorLogger::callStackToString(callStack) << ": (" << severity << ") " << msg;
+        reportErr(text.str());
+    }
 }

src/cppcheckexecutor.h

@@ -54,15 +54,7 @@ public:
      */
     unsigned int check(int argc, const char* const argv[]);
 
-    /**
+
-     * Errors and warnings are directed here. This should be
-     * called by the CppCheck class only.
-     *
-     * @param errmsg Errors messages are normally in format
-     * "[filepath:line number] Message", e.g.
-     * "[main.cpp:4] Uninitialized member variable"
-     */
-    virtual void reportErr(const std::string &errmsg);
 
     /**
      * Information about progress is directed here. This should be
@@ -73,7 +65,17 @@ public:
     virtual void reportOut(const std::string &outmsg);
 
     /** xml output of errors */
-    virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg);
+    virtual void reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg);
+
+private:
+
+    /**
+     * Helper function to print out errors. Appends a line change.
+     * @param errmsg, string to printed to error stream
+     */
+    void reportErr(const std::string &errmsg);
+
+    bool _useXML;
 };
 
 #endif // CPPCHECKEXECUTOR_H

src/errorlogger.cpp

@@ -32,20 +32,19 @@ void ErrorLogger::_writemsg(const Tokenizer *tokenizer, const Token *tok, const
 
 void ErrorLogger::_writemsg(const Tokenizer *tokenizer, const std::list<const Token *> &callstack, const char severity[], const std::string msg, const std::string &id)
 {
-    // Todo.. callstack handling
+    std::list<FileLocation> locationList;
-    const std::string &file(tokenizer->getFiles()->at(callstack.back()->fileIndex()));
+    for (std::list<const Token *>::const_iterator tok = callstack.begin(); tok != callstack.end(); ++tok)
-    std::ostringstream linenr;
+    {
-    linenr << callstack.back()->linenr();
+        FileLocation loc;
-    reportXml(file,
+        loc.file = tokenizer->file(*tok);
-              linenr.str(),
+        loc.line = (*tok)->linenr();
+        locationList.push_back(loc);
+    }
+
+    reportErr(locationList,
               id,
               severity,
               msg);
-
-    std::ostringstream ostr;
-    for (std::list<const Token *>::const_iterator tok = callstack.begin(); tok != callstack.end(); ++tok)
-        ostr << (tok == callstack.begin() ? "" : " -> ") << tokenizer->fileLine(*tok);
-    reportErr(ostr.str() + ": (" + severity + ") " + msg);
 }
 
 
@@ -57,5 +56,15 @@ void ErrorLogger::_writemsg(const std::string msg, const std::string &id)
     xml << " msg=\"" << msg << "\"";
     xml << ">";
 
-    reportErr(msg);
+    std::list<FileLocation> empty;
+    empty.push_back(FileLocation());
+    reportErr(empty, "id", "severity", msg);
+}
+
+std::string ErrorLogger::callStackToString(const std::list<FileLocation> &callStack)
+{
+    std::ostringstream ostr;
+    for (std::list<FileLocation>::const_iterator tok = callStack.begin(); tok != callStack.end(); ++tok)
+        ostr << (tok == callStack.begin() ? "" : " -> ") << "[" << (*tok).file << ":" << (*tok).line << "]";
+    return ostr.str();
 }

src/errorlogger.h

@@ -27,6 +27,16 @@
 class Token;
 class Tokenizer;
 
+/**
+ * File name and line number.
+ */
+class FileLocation
+{
+public:
+    std::string file;
+    unsigned int line;
+};
+
 /**
  * This is an interface, which the class responsible of error logging
  * should implement.
@@ -38,15 +48,6 @@ public:
     ErrorLogger() { }
     virtual ~ErrorLogger() { }
 
-    /**
-     * Errors and warnings are directed here.
-     *
-     * @param errmsg Errors messages are normally in format
-     * "[filepath:line number] Message", e.g.
-     * "[main.cpp:4] Uninitialized member variable"
-     */
-    virtual void reportErr(const std::string &errmsg) = 0;
-
     /**
      * Information about progress is directed here.
      *
@@ -55,16 +56,17 @@ public:
     virtual void reportOut(const std::string &outmsg) = 0;
 
     /**
-     * XML output of error / warning
+     * Output of error / warning
      * Todo: callstack handling
      *
-     * @param file      filepath (can be "")
+     * @param callStack File names and line numbers where the error
-     * @param line      line (can be "")
+     * was found and where it was called from. Error location is last
+     * element in the list
      * @param id        error id (function name)
      * @param severity  severity of error (always, all, style, all+style, never)
      * @param msg       error message in plain text
      */
-    virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg) = 0;
+    virtual void reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg) = 0;
 
     void arrayIndexOutOfBounds(const Tokenizer *tokenizer, const std::list<const Token *> &Location)
     {
@@ -381,6 +383,35 @@ public:
         return true;
     }
 
+    void dangerousFunctionmktemp(const Tokenizer *tokenizer, const Token *Location)
+    {
+        _writemsg(tokenizer, Location, "style", "Found 'mktemp'. You should use 'mkstemp' instead", "dangerousFunctionmktemp");
+    }
+    static bool dangerousFunctionmktemp(const Settings &s)
+    {
+        return s._checkCodingStyle;
+    }
+
+    void dangerousFunctiongets(const Tokenizer *tokenizer, const Token *Location)
+    {
+        _writemsg(tokenizer, Location, "style", "Found 'gets'. You should use 'fgets' instead", "dangerousFunctiongets");
+    }
+    static bool dangerousFunctiongets(const Settings &s)
+    {
+        return s._checkCodingStyle;
+    }
+
+    void dangerousFunctionscanf(const Tokenizer *tokenizer, const Token *Location)
+    {
+        _writemsg(tokenizer, Location, "style", "Found 'scanf'. You should use 'fgets' instead", "dangerousFunctionscanf");
+    }
+    static bool dangerousFunctionscanf(const Settings &s)
+    {
+        return s._checkCodingStyle;
+    }
+
+
+    static std::string callStackToString(const std::list<FileLocation> &callStack);
 
 private:
     void _writemsg(const Tokenizer *tokenizer, const Token *tok, const char severity[], const std::string msg, const std::string &id);

src/tokenize.cpp

@@ -1740,6 +1740,9 @@ std::string Tokenizer::fileLine(const Token *tok) const
     return ostr.str();
 }
 
-
+std::string Tokenizer::file(const Token *tok) const
+{
+    return _files.at(tok->fileIndex());
+}
 
 //---------------------------------------------------------------------------

src/tokenize.h

@@ -75,6 +75,8 @@ public:
     const Token *GetFunctionTokenByName(const char funcname[]) const;
     const Token *tokens() const;
 
+    std::string file(const Token *tok) const;
+
 protected:
 
     /** Add braces to an if-block

test/testdangerousfunctions.cpp

@@ -75,7 +75,7 @@ private:
               "{\n"
               "    char *x = mktemp(\"/tmp/zxcv\");\n"
               "}\n");
-        ASSERT_EQUALS(std::string("[test.cpp:3]: Found 'mktemp'. You should use 'mkstemp' instead\n"), errout.str());
+        ASSERT_EQUALS(std::string("[test.cpp:3]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n"), errout.str());
     }
 
     void testgets()
@@ -84,7 +84,7 @@ private:
               "{\n"
               "    char *x = gets();\n"
               "}\n");
-        ASSERT_EQUALS(std::string("[test.cpp:3]: Found 'gets'. You should use 'fgets' instead\n"), errout.str());
+        ASSERT_EQUALS(std::string("[test.cpp:3]: (style) Found 'gets'. You should use 'fgets' instead\n"), errout.str());
     }
 
     void testscanf()
@@ -94,7 +94,7 @@ private:
               "    char *x;\n"
               "    scanf(\"%s\", x);\n"
               "}\n");
-        ASSERT_EQUALS(std::string("[test.cpp:4]: Found 'scanf'. You should use 'fgets' instead\n"), errout.str());
+        ASSERT_EQUALS(std::string("[test.cpp:4]: (style) Found 'scanf'. You should use 'fgets' instead\n"), errout.str());
     }
 
 };

test/testsuite.cpp

@@ -167,18 +167,14 @@ void TestFixture::runTests(const char cmd[])
     std::cerr << errmsg.str();
 }
 
-
-void TestFixture::reportErr(const std::string &errmsg)
-{
-    errout << errmsg << std::endl;
-}
-
 void TestFixture::reportOut(const std::string & /*outmsg*/)
 {
     // These can probably be ignored
 }
 
-void TestFixture::reportXml(const std::string & /*file*/, const std::string & /*line*/, const std::string & /*id*/, const std::string & /*severity*/, const std::string & /*msg*/)
+void TestFixture::reportErr(const std::list<FileLocation> &callStack, const std::string & /*id*/, const std::string &severity, const std::string &msg)
 {
-    // These can probably be ignored
+    std::ostringstream text;
+    text << ErrorLogger::callStackToString(callStack) << ": (" << severity << ") " << msg;
+    errout << text.str() << std::endl;
 }

test/testsuite.h

@@ -42,9 +42,8 @@ protected:
     void assertEquals(const char *filename, int linenr, unsigned int expected, unsigned int actual);
 
 public:
-    virtual void reportErr(const std::string &errmsg);
     virtual void reportOut(const std::string &outmsg);
-    virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg);
+    virtual void reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg);
 
     void run(const std::string &str);
 

tools/errmsg.cpp

@@ -105,6 +105,12 @@ int main()
     err.push_back(Message("strPlusChar", Message::error, "Unusual pointer arithmetic"));
     err.push_back(Message("returnLocalVariable", Message::error, "Returning pointer to local array variable"));
 
+    // checkdangerousfunctions.cpp..
+    err.push_back(Message("dangerousFunctionmktemp", Message::style, "Found 'mktemp'. You should use 'mkstemp' instead"));
+    err.push_back(Message("dangerousFunctiongets", Message::style, "Found 'gets'. You should use 'fgets' instead"));
+    err.push_back(Message("dangerousFunctionscanf", Message::style, "Found 'scanf'. You should use 'fgets' instead"));
+
+
     // Generate code..
     std::cout << "Generate code.." << std::endl;
     std::ofstream fout("errorlogger.h");
@@ -137,6 +143,16 @@ int main()
     fout << "class Tokenizer;\n";
     fout << "\n";
     fout << "/**\n";
+    fout << " * File name and line number.\n";
+    fout << " */\n";
+    fout << "class FileLocation\n";
+    fout << "{\n";
+    fout << "public:\n";
+    fout << "    std::string file;\n";
+    fout << "    unsigned int line;\n";
+    fout << "};\n";
+    fout << "\n";
+    fout << "/**\n";
     fout << " * This is an interface, which the class responsible of error logging\n";
     fout << " * should implement.\n";
     fout << " */\n";
@@ -148,15 +164,6 @@ int main()
     fout << "    virtual ~ErrorLogger() { }\n";
     fout << "\n";
     fout << "    /**\n";
-    fout << "     * Errors and warnings are directed here.\n";
-    fout << "     *\n";
-    fout << "     * @param errmsg Errors messages are normally in format\n";
-    fout << "     * \"[filepath:line number] Message\", e.g.\n";
-    fout << "     * \"[main.cpp:4] Uninitialized member variable\"\n";
-    fout << "     */\n";
-    fout << "    virtual void reportErr(const std::string &errmsg) = 0;\n";
-    fout << "\n";
-    fout << "    /**\n";
     fout << "     * Information about progress is directed here.\n";
     fout << "     *\n";
     fout << "     * @param outmsg, E.g. \"Checking main.cpp...\"\n";
@@ -164,20 +171,24 @@ int main()
     fout << "    virtual void reportOut(const std::string &outmsg) = 0;\n";
     fout << "\n";
     fout << "    /**\n";
-    fout << "     * XML output of error / warning\n";
+    fout << "     * Output of error / warning\n";
     fout << "     * Todo: callstack handling\n";
     fout << "     *\n";
-    fout << "     * @param file      filepath (can be \"\")\n";
+    fout << "     * @param callStack File names and line numbers where the error\n";
-    fout << "     * @param line      line (can be \"\")\n";
+    fout << "     * was found and where it was called from. Error location is last\n";
+    fout << "     * element in the list\n";
     fout << "     * @param id        error id (function name)\n";
     fout << "     * @param severity  severity of error (always, all, style, all+style, never)\n";
     fout << "     * @param msg       error message in plain text\n";
     fout << "     */\n";
-    fout << "    virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg) = 0;\n";
+    fout << "    virtual void reportErr(const std::list<FileLocation> &callStack, const std::string &id, const std::string &severity, const std::string &msg) = 0;\n";
     fout << "\n";
 
     for (std::list<Message>::const_iterator it = err.begin(); it != err.end(); ++it)
         it->generateCode(fout);
+
+    fout << "\n";
+    fout << "    static std::string callStackToString(const std::list<FileLocation> &callStack);\n";
     fout << "\n";
     fout << "private:\n";
     fout << "    void _writemsg(const Tokenizer *tokenizer, const Token *tok, const char severity[], const std::string msg, const std::string &id);\n";