Additional Strutil tests (#3655)

Better coverage of edge cases for escape_chars/unescape_chars, stoi
hex conversion, strtod, stod, freestanding c_str(), parse_nested
failure cases, base64_encode, debug.

Author: lgritz

.github/workflows/analysis.yml

@@ -15,6 +15,7 @@ on:
     branches:
       - master
       - '*analysis*'
+      - '*sonar*'
   # Allow manual kicking off of the workflow from github.com
   workflow_dispatch:
   # Uncomment the following line if we want to run analysis on all PRs:

src/libutil/strutil.cpp

@@ -357,6 +357,7 @@ Strutil::escape_chars(string_view unescaped)
             case '\r': c = 'r'; break;
             case '\f': c = 'f'; break;
             case '\a': c = 'a'; break;
+            default: break;
             }
             s.insert(i, 1, c);
         }
@@ -384,10 +385,8 @@ Strutil::unescape_chars(string_view escaped)
                 case 'b': s[i] = '\b'; break;
                 case 'r': s[i] = '\r'; break;
                 case 'f': s[i] = '\f'; break;
-                case 'a':
-                    s[i] = '\a';
-                    break;
-                    // default case: the deletion is enough (backslash and quote)
+                case 'a': s[i] = '\a'; break;
+                default: break;  // the deletion is enough (backslash and quote)
                 }
             } else if (c >= '0' && c < '8') {
                 // up to 3 octal digits
@@ -1620,7 +1619,6 @@ Strutil::strtod(const char* nptr, char** endptr) noexcept
     // Can use strtod_l on platforms that support it
 #if defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) \
     || defined(__FreeBSD_kernel__) || defined(__GLIBC__)
-    // static initialization inside function is thread-safe by C++11 rules!
     return strtod_l(nptr, endptr, c_loc);
 #elif defined(_WIN32)
     // Windows has _strtod_l
@@ -1700,7 +1698,7 @@ Strutil::stof(string_view s, size_t* pos)
     // will use the "short string optimization", meaning that this string
     // creation will NOT need an allocation/free for most strings we expect
     // to hold a text representation of a float.
-    return Strutil::stof(std::string(s).c_str(), pos);
+    return Strutil::stof(std::string(s), pos);
 }
 
 

src/libutil/strutil_test.cpp

@@ -305,8 +305,9 @@ test_escape(string_view raw, string_view escaped)
 void
 test_escape_sequences()
 {
-    test_escape ("\\ \n \r \t", "\\\\ \\n \\r \\t");
+    test_escape ("\\ \n \r \t \v \b \f \a", "\\\\ \\n \\r \\t \\v \\b \\f \\a");
     test_escape (" \"quoted\" ",  " \\\"quoted\\\" ");
+    OIIO_CHECK_EQUAL(Strutil::unescape_chars("A\\023B"), "A\023B");
 }
 
 
@@ -852,6 +853,7 @@ test_numeric_conversion()
                      std::numeric_limits<int>::max());
     OIIO_CHECK_EQUAL(Strutil::stoi("-12345678901234567890"),
                      std::numeric_limits<int>::min());
+    OIIO_CHECK_EQUAL(Strutil::stoi("0x100", nullptr, 16), 256);  // hex
 
     OIIO_CHECK_EQUAL(Strutil::stoui("hi"), 0);
     OIIO_CHECK_EQUAL(Strutil::stoui("  "), 0);
@@ -921,6 +923,22 @@ test_numeric_conversion()
     OIIO_CHECK_EQUAL (Strutil::stof("100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001E-200"), 1.0f);
     OIIO_CHECK_EQUAL (Strutil::stof("0.00000000000000000001"), 1.0e-20f);
 
+    OIIO_CHECK_EQUAL(Strutil::strtod("314.25"), 314.25);
+    OIIO_CHECK_EQUAL(Strutil::strtod("hi"), 0.0);
+
+    pos = 100;
+    OIIO_CHECK_EQUAL(Strutil::stod(string_view("314.25"), &pos), 314.25);
+    OIIO_CHECK_EQUAL(pos, 6);
+    pos = 100;
+    OIIO_CHECK_EQUAL(Strutil::stod("hi", &pos), 0.0);
+    OIIO_CHECK_EQUAL(pos, 0);
+    pos = 100;
+    OIIO_CHECK_EQUAL(Strutil::stod("", &pos), 0.0);
+    OIIO_CHECK_EQUAL(pos, 0);
+    pos = 100;
+    OIIO_CHECK_EQUAL(Strutil::stod(nullptr, &pos), 0.0);
+    OIIO_CHECK_EQUAL(pos, 0);
+
     // Note: we don't test from_strings<> separately because it's just
     // implemented directly as calls to stoi, stoui, stof.
 
@@ -1249,6 +1267,17 @@ test_string_view()
     // not horribly broken.
     wstring_view wsv;
     OIIO_CHECK_ASSERT(wsv == wsv);
+
+    // Test the freestanding OIIO::c_str() function
+    OIIO_CHECK_EQUAL(OIIO::c_str(""), std::string());
+    OIIO_CHECK_EQUAL(OIIO::c_str(cstr), cstr);
+    OIIO_CHECK_EQUAL(OIIO::c_str(s), s);
+    OIIO_CHECK_EQUAL(OIIO::c_str(ustring(cstr)), ustring(cstr));
+    OIIO_CHECK_EQUAL(OIIO::c_str(sr), sr);
+    OIIO_CHECK_EQUAL(OIIO::c_str(string_view(sr.data(), 2)), std::string("01"));
+    Strutil::print("addr cstr={:p}, s={:p}, ustring={:p}, sr={:p}, c_str(sr)={:p}\n",
+                     (void*)cstr, (void*)s.c_str(), (void*)ustring(cstr).c_str(), (void*)sr.data(),
+                     (void*)c_str(sr));
 }
 
 
@@ -1412,8 +1441,16 @@ void test_parse ()
 
     s = "[a([b]c)]x]"; ss = parse_nested (s);
     OIIO_CHECK_EQUAL (ss, "[a([b]c)]"); OIIO_CHECK_EQUAL (s, "x]");
-    s = "[a([b]c)]x]"; ss = parse_nested (s, false);
+    s = "[a([b]c)]x]"; ss = parse_nested (s, false);  // no eating
     OIIO_CHECK_EQUAL (ss, "[a([b]c)]"); OIIO_CHECK_EQUAL (s, "[a([b]c)]x]");
+    s = "([a([b]c)])x]"; ss = parse_nested (s);
+    OIIO_CHECK_EQUAL (ss, "([a([b]c)])"); OIIO_CHECK_EQUAL (s, "x]");
+    s = "blah[a([b]c)]x]"; ss = parse_nested (s);
+    OIIO_CHECK_EQUAL (ss, ""); OIIO_CHECK_EQUAL (s, "blah[a([b]c)]x]");
+    s = ""; ss = parse_nested (s);
+    OIIO_CHECK_EQUAL (ss, ""); OIIO_CHECK_EQUAL (s, "");
+    s = "(blah"; ss = parse_nested (s);
+    OIIO_CHECK_EQUAL (ss, ""); OIIO_CHECK_EQUAL (s, "(blah");
 }
 
 
@@ -1581,6 +1618,14 @@ test_edit_distance()
 
 
 
+void
+test_base64_encode()
+{
+    OIIO_CHECK_EQUAL(Strutil::base64_encode("foo123,()"), "Zm9vMTIzLCgp");
+}
+
+
+
 int
 main(int /*argc*/, char* /*argv*/[])
 {
@@ -1615,6 +1660,9 @@ main(int /*argc*/, char* /*argv*/[])
     test_string_compare_function();
     test_datetime();
     test_edit_distance();
+    test_base64_encode();
+
+    Strutil::debug("debug message\n");
 
     return unit_test_failures;
 }

src/libutil/ustring.cpp

@@ -602,15 +602,15 @@ ustring::concat(string_view s, string_view t)
 std::string
 ustring::getstats(bool verbose)
 {
-    UstringTable& table(ustring_table());
     std::ostringstream out;
     out.imbue(std::locale::classic());  // Force "C" locale with '.' decimal
-    size_t n_e = table.get_num_entries();
-    size_t mem = table.get_memory_usage();
+    size_t n_e = total_ustrings();
+    size_t mem = memory();
     if (verbose) {
         out << "ustring statistics:\n";
 #ifdef USTRING_TRACK_NUM_LOOKUPS
-        out << "  ustring requests: " << table.get_num_lookups() << "\n";
+        out << "  ustring requests: " << ustring_table().get_num_lookups()
+            << "\n";
 #endif
         out << "  unique strings: " << n_e << "\n";
         out << "  ustring memory: " << Strutil::memformat(mem) << "\n";
@@ -625,7 +625,7 @@ ustring::getstats(bool verbose)
 #endif
     } else {
 #ifdef USTRING_TRACK_NUM_LOOKUPS
-        out << "requests: " << table.get_num_lookups() << ", ";
+        out << "requests: " << ustring_table().get_num_lookups() << ", ";
 #endif
         out << "unique " << n_e << ", " << Strutil::memformat(mem);
     }

src/libutil/ustring_test.cpp

@@ -143,6 +143,7 @@ test_ustring()
     // make_unique, is_unique, from_unique
     const char* foostr = foo.c_str();
     OIIO_CHECK_EQUAL(ustring::make_unique("foo"), foostr);
+    OIIO_CHECK_EQUAL(ustring::make_unique(string_view()), ustring());
     OIIO_CHECK_EQUAL(ustring::is_unique(foostr), true);
     OIIO_CHECK_EQUAL(ustring::is_unique("foo"), false);
     OIIO_CHECK_EQUAL(ustring::from_unique(foostr), foo);
@@ -292,8 +293,7 @@ main(int argc, char* argv[])
     benchmark_threaded_ustring_creation();
     verify_no_collisions();
 
-    if (verbose)
-        std::cout << "\n" << ustring::getstats() << "\n";
+    std::cout << "\n" << ustring::getstats(true) << "\n";
 
     return unit_test_failures;
 }