Fix #3243 (Improve non reentrant function check)

- Use std::map instead of linear std::list walk
  and run fast tests like tok->isName() first.

  Global speed up is 4.8% (profiled with google-perftools)

- Catch function invocations in global namespace
  and ignore other namespaces except "std".

  std::localtime() and others are also non-thread safe on POSIX.

  Note: The check matches f.e. also on "std::getrpcbyname()",
  but that would result in a compile error anyway.
  No need to have an extra "std::xxxxx" whitelist.

- Remove double listed "rand" and "getrpcbyname" function names

lib/checknonreentrantfunctions.cpp

@@ -39,16 +39,31 @@ void CheckNonReentrantFunctions::nonReentrantFunctions()
     if (_tokenizer->isJavaOrCSharp())
         return;
 
+    std::map<std::string,std::string>::const_iterator nonReentrant_end = _nonReentrantFunctions.end();
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
-        std::list< std::pair<const std::string, const std::string> >::const_iterator it(_nonReentrantFunctions.begin()), itend(_nonReentrantFunctions.end());
-        for (; it!=itend; ++it) {
-            if (tok->strAt(1) == it->first && tok->strAt(2) == "(" && tok->tokAt(1)->varId() == 0 && !tok->tokAt(0)->isName() && !Token::Match(tok, ".|::|:|,")) {
-                // If checking code that is single threaded, this might be not interesing for all.
-                // Therefore this is "portabiblity"
-                reportError(tok->tokAt(1), Severity::portability, "nonreentrantFunctions"+it->first, it->second);
-                break;
-            }
+        // Look for function invocations
+        if (!tok->isName() || tok->strAt(1) != "(" || tok->varId() != 0)
+            continue;
+
+        // Check for non-reentrant function name
+        std::map<std::string,std::string>::const_iterator it = _nonReentrantFunctions.find(tok->str());
+        if (it == nonReentrant_end)
+            continue;
+
+        const Token *prev = tok->previous();
+        if (prev)
+        {
+            // Ignore function definitions, class members or class definitions
+            if (prev->isName() || Token::Match(prev, ".|:"))
+                continue;
+
+            // Check for "std" or global namespace, ignore other namespaces
+            if (prev->str() == "::" && prev->previous() && prev->previous()->str() != "std" && prev->previous()->isName())
+                continue;
         }
+
+        // Only affecting multi threaded code, therefore this is "portability"
+        reportError(tok, Severity::portability, "nonreentrantFunctions"+it->first, it->second);
     }
 }
 //---------------------------------------------------------------------------

lib/checknonreentrantfunctions.h

@@ -24,7 +24,7 @@
 
 #include "check.h"
 #include <string>
-#include <list>
+#include <map>
 
 
 /// @addtogroup Checks
@@ -58,7 +58,7 @@ public:
 private:
 
     /* function name / error message */
-    std::list< std::pair< const std::string, const std::string> > _nonReentrantFunctions;
+    std::map<std::string,std::string> _nonReentrantFunctions;
 
     /** init nonreentrant functions list ' */
     void initNonReentrantFunctions() {
@@ -66,8 +66,8 @@ private:
             "ctime", "localtime", "gmtime", "asctime", "strtok", "gethostbyname", "gethostbyaddr", "getservbyname"
             , "getservbyport", "crypt", "ttyname", "rand", "gethostbyname2"
             , "getprotobyname", "getnetbyname", "getnetbyaddr", "getrpcbyname", "getrpcbynumber", "getrpcent"
-            , "rand", "ctermid", "tmpnam", "readdir", "getlogin", "getpwent", "getpwnam", "getpwuid", "getspent"
-            , "fgetspent", "getspnam", "getgrnam", "getgrgid", "getnetgrent", "getrpcbyname", "tempnam", "fgetpwent"
+            , "ctermid", "tmpnam", "readdir", "getlogin", "getpwent", "getpwnam", "getpwuid", "getspent"
+            , "fgetspent", "getspnam", "getgrnam", "getgrgid", "getnetgrent", "tempnam", "fgetpwent"
             , "fgetgrent", "ecvt", "gcvt", "getservent", "gethostent", "getgrent", "fcvt"
         };
 
@@ -78,14 +78,14 @@ private:
             strMsg+= "\'. For threadsafe applications it is recommended to use the reentrant replacement function \'";
             strMsg+=non_reentrant_functions_list[i];
             strMsg+="_r\'";
-            _nonReentrantFunctions.push_back(std::make_pair(non_reentrant_functions_list[i],strMsg));
+            _nonReentrantFunctions[non_reentrant_functions_list[i]] = strMsg;
         }
     }
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
         CheckNonReentrantFunctions c(0, settings, errorLogger);
 
-        std::list< std::pair<const std::string, const std::string> >::const_iterator it(_nonReentrantFunctions.begin()), itend(_nonReentrantFunctions.end());
+        std::map<std::string,std::string>::const_iterator it(_nonReentrantFunctions.begin()), itend(_nonReentrantFunctions.end());
         for (; it!=itend; ++it) {
             c.reportError(0, Severity::portability, "nonreentrantFunctions"+it->first, it->second);
         }
@@ -97,7 +97,7 @@ private:
 
     std::string classInfo() const {
         std::string info = "Warn if any of these non reentrant functions are used:\n";
-        std::list< std::pair<const std::string, const std::string> >::const_iterator it(_nonReentrantFunctions.begin()), itend(_nonReentrantFunctions.end());
+        std::map<std::string,std::string>::const_iterator it(_nonReentrantFunctions.begin()), itend(_nonReentrantFunctions.end());
         for (; it!=itend; ++it) {
             info += "* " + it->first + "\n";
         }

test/testnonreentrantfunctions.cpp

@@ -34,6 +34,7 @@ private:
 
     void run() {
         TEST_CASE(test_crypt);
+        TEST_CASE(test_namespace_handling);
     }
 
     void check(const char code[]) {
@@ -85,8 +86,57 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void test_namespace_handling() {
+        check("int f()\n"
+              "{\n"
+              "    time_t t = 0;"
+              "    std::localtime(&t);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (portability) Found non reentrant function 'localtime'. For threadsafe applications it is recommended to use the reentrant replacement function 'localtime_r'\n", errout.str());
+
+        // Passed as function argument
+        check("int f()\n"
+              "{\n"
+              "    printf(\"Magic guess: %d\n\", rand());\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (portability) Found non reentrant function 'rand'. For threadsafe applications it is recommended to use the reentrant replacement function 'rand_r'\n", errout.str());
+
+        // Pass return value
+        check("int f()\n"
+              "{\n"
+              "    time_t t = 0;"
+              "    struct tm *foo = localtime(&t);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (portability) Found non reentrant function 'localtime'. For threadsafe applications it is recommended to use the reentrant replacement function 'localtime_r'\n", errout.str());
+
+        // Access via global namespace
+        check("int f()\n"
+              "{\n"
+              "    ::rand();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (portability) Found non reentrant function 'rand'. For threadsafe applications it is recommended to use the reentrant replacement function 'rand_r'\n", errout.str());
+
+        // Be quiet on function definitions
+        check("int rand()\n"
+              "{\n"
+              "    return 123;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // Be quiet on other namespaces
+        check("int f()\n"
+              "{\n"
+              "    foobar::rand();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // Be quiet on class member functions
+        check("int f()\n"
+              "{\n"
+              "    foobar.rand();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestNonReentrantFunctions)
-
-