Library: simplified code and added test cases for validating <valid>-tag expressions

Author: orbitcowboy

lib/library.cpp

@@ -708,46 +708,11 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
                 else if (argnodename == "valid") {
                     // Validate the validation expression
                     const char *p = argnode->GetText();
-                    bool error = false;
-                    bool range = false;
-                    bool has_dot = false;
-                    bool has_E = false;
-
-                    if (!p)
-                        return Error(BAD_ATTRIBUTE_VALUE, "\"\"");
-
-                    error = *p == '.';
-                    for (; *p; p++) {
-                        if (std::isdigit(*p))
-                            error |= (*(p+1) == '-');
-                        else if (*p == ':') {
-                            error |= range | (*(p+1) == '.');
-                            range = true;
-                            has_dot = false;
-                            has_E = false;
-                        } else if (*p == '-')
-                            error |= (!std::isdigit(*(p+1)));
-                        else if (*p == ',') {
-                            range = false;
-                            error |= *(p+1) == '.';
-                            has_dot = false;
-                            has_E = false;
-                        } else if (*p == '.') {
-                            error |= has_dot | (!std::isdigit(*(p+1)));
-                            has_dot = true;
-                        } else if (*p == 'E' || *p == 'e') {
-                            error |= has_E;
-                            has_E = true;
-                        } else
-                            error = true;
-                    }
-                    if (error)
-                        return Error(BAD_ATTRIBUTE_VALUE, argnode->GetText());
-
+                    if (!isCompliantValidationExpression(p))
+                        return Error(BAD_ATTRIBUTE_VALUE, (!p ? "\"\"" : argnode->GetText()));
                     // Set validation expression
                     ac.valid = argnode->GetText();
                 }
-
                 else if (argnodename == "minsize") {
                     const char *typeattr = argnode->Attribute("type");
                     if (!typeattr)
@@ -1242,6 +1207,48 @@ const Library::WarnInfo* Library::getWarnInfo(const Token* ftok) const
     return &i->second;
 }
 
+bool Library::isCompliantValidationExpression(const char* p)
+{
+    if (!p)
+        return false;
+
+    bool error = false;
+    bool range = false;
+    bool has_dot = false;
+    bool has_E = false;
+
+    error = *p == '.';
+    for (; *p; p++) {
+        if (std::isdigit(*p))
+            error |= (*(p + 1) == '-');
+        else if (*p == ':') {
+            error |= range | (*(p + 1) == '.');
+            range = true;
+            has_dot = false;
+            has_E = false;
+        }
+        else if ((*p == '-')|| (*p == '+'))
+            error |= (!std::isdigit(*(p + 1)));
+        else if (*p == ',') {
+            range = false;
+            error |= *(p + 1) == '.';
+            has_dot = false;
+            has_E = false;
+        }
+        else if (*p == '.') {
+            error |= has_dot | (!std::isdigit(*(p + 1)));
+            has_dot = true;
+        }
+        else if (*p == 'E' || *p == 'e') {
+            error |= has_E;
+            has_E = true;
+        }
+        else
+            return false;
+    }
+    return !error;
+}
+
 bool Library::formatstr_function(const Token* ftok) const
 {
     if (isNotLibraryFunction(ftok))

lib/library.h

@@ -142,6 +142,8 @@ class CPPCHECKLIB Library {
         mNoReturn[funcname] = noreturn;
     }
 
+    static bool isCompliantValidationExpression(const char* p);
+
     /** is allocation type memory? */
     static bool ismemory(const int id) {
         return ((id > 0) && ((id & 1) == 0));

test/testlibrary.cpp

@@ -40,6 +40,7 @@ class TestLibrary : public TestFixture {
     Settings settings;
 
     void run() OVERRIDE {
+        TEST_CASE(isCompliantValidationExpression);
         TEST_CASE(empty);
         TEST_CASE(function);
         TEST_CASE(function_match_scope);
@@ -72,6 +73,26 @@ class TestLibrary : public TestFixture {
         return library.load(doc);
     }
 
+    void isCompliantValidationExpression()
+    {
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1:"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression(":1"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1,42"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1,-42"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1.0:42.0"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1.175494e-38:3.402823e+38"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1.175494e-38,3.402823e+38"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1.175494e-38:"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression(":1.175494e-38"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression(":42.0"));
+
+        // Robustness tests
+        ASSERT_EQUALS(false, Library::isCompliantValidationExpression(nullptr));
+        ASSERT_EQUALS(false, Library::isCompliantValidationExpression("x"));
+    }
+
     void empty() const {
         // Reading an empty library file is considered to be OK
         const char xmldata[] = "<?xml version=\"1.0\"?>\n<def/>";