From: Thomas Markwalder <tmark@isc.org>
Date: Thu, 4 Jun 2026 18:11:36 +0000 (-0400)
Subject: [#4393] Applied suggested changes
X-Git-Tag: Kea-3.3.0~135
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4ec0fa8895fc83cc1022f91a729d8f6f3d98e8a5;p=thirdparty%2Fkea.git

[#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 012121dd71..667da1639f 100644
--- 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 c66c495a9f..45327e6eb3 100644
--- 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 95fa8b6cf5..5c6dfbe2e2 100644
--- 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