[#4393] Applied suggested changes

modified:   src/lib/util/csv_file.cc
modified:   src/lib/util/tests/csv_file_unittest.cc
modified:   src/lib/util/tests/str_unittests.cc

diff --git a/src/lib/util/csv_file.cc b/src/lib/util/csv_file.cc
index 012121dd7177f2cb8694730358809a213650ce07..667da1639fb1e21df875009c2dcd95d55f3ca32c 100644 (file)
--- a/src/lib/util/csv_file.cc
+++ b/src/lib/util/csv_file.cc
@@ -446,42 +446,38 @@ CSVFile::validateHeader(const CSVRow& header) {
 
 const std::string CSVRow::escape_tag("&#x");
 
-// Macro used in check if a character should be escaped.
-// Safe chars are less than 127 and have any of two bits
-// in 0x60 set. This is faster than isprint().
-#define ESCAPE_IT(c,s,e) (c > 0x7e || !(c & 0x60) || c == s || c == e)
-
 std::string
 CSVRow::escapeCharacters(const std::string& orig_str, const char separator) {
-    auto org_cstr = orig_str.c_str();
-    const char* orgpos;
-
-    // Most of time we probably don't need escape to escape anything, so do
-    // a quick scan to see if anything needs escaping. For things like user-context
-    // we'll find a comma pretty quickly.
-    for ( orgpos = &org_cstr[0] ; *orgpos ; ++orgpos) {
-        if (ESCAPE_IT(*orgpos, separator, escape_tag[0])) {
-            break;
-        }
-    }
+    auto escape_it = [](char c, char s, char e) -> bool {
+        return ((c < 0x20) || (c > 0x7e) || c == s || c == e);
+    };
 
-    if (!*orgpos) {
-        // Nothing to escape, return the original.
-        return (orig_str);
-    }
+    // Count the number of needed escapes.
+    size_t escapes = 0;
+    for (char c : orig_str) {
+        if (escape_it(c, separator, escape_tag[0])) {
+            ++escapes;
+         }
+     }
 
-    // Iterate over the original string, escaped chars that need it.
+    if (escapes == 0) {
+         // Nothing to escape, return the original.
+         return (orig_str);
+     }
+
+    // Make the result large enough to avoid reallocations.
     std::string esc_str;
-    for ( orgpos = &org_cstr[0] ; *orgpos ; ++orgpos) {
-        if (ESCAPE_IT(*orgpos, separator, escape_tag[0])) {
-            esc_str.append(escape_tag);
-            esc_str.append(str::byteToHex(*orgpos));
-        } else {
-            esc_str.append(1, *orgpos);
-        }
-    }
+    esc_str.reserve(orig_str.size() + escapes * (escape_tag.size() + 1));
+    // Iterate over the original string, escaped chars that need it.
+    for (char c : orig_str) {
+        if (escape_it(c, separator, escape_tag[0])) {
+             esc_str.append(escape_tag);
+            esc_str.append(str::byteToHex(c));
+         } else {
+            esc_str.push_back(c);
+         }
+     }
 
-    // Return the escapd string.
     return (esc_str);
 }
 

diff --git a/src/lib/util/tests/csv_file_unittest.cc b/src/lib/util/tests/csv_file_unittest.cc
index c66c495a9f7265184654bca2a51ff8d1c1f0d47c..45327e6eb3dbef0ea6a12110b3f98cc6dca4893e 100644 (file)
--- a/src/lib/util/tests/csv_file_unittest.cc
+++ b/src/lib/util/tests/csv_file_unittest.cc
@@ -45,7 +45,6 @@ TEST(CSVRowTest, escapeUnescape) {
     auto escaped = CSVRow::escapeCharacters(org, ',');
 
     ASSERT_EQ(escaped, expected_escaped);
-    std::cout << "escaped: " << escaped << std::endl;
 
     // Unescape it and make sure we get the original back.
     auto unescaped = CSVRow::unescapeCharacters(escaped);

diff --git a/src/lib/util/tests/str_unittests.cc b/src/lib/util/tests/str_unittests.cc
index 95fa8b6cf5197d974cd177c1bff3abfc18384f03..5c6dfbe2e29404982d2432a64ce75f8ef03230c2 100644 (file)
--- a/src/lib/util/tests/str_unittests.cc
+++ b/src/lib/util/tests/str_unittests.cc
@@ -581,4 +581,13 @@ TEST_F(StringUtilTest, printOrDump) {
     }
 }
 
+// Verifies the byteToHex tool.
+TEST_F(StringUtilTest, byteToHex) {
+    for (unsigned i = 0; i < 256; i++) {
+        std::ostringstream ss;
+        ss << std::hex << std::setw(2) << std::setfill('0') << i;
+        EXPECT_EQ(ss.str(), byteToHex(i));
+    }
+}
+
 }  // namespace