From: Florian Forster <octo@collectd.org>
Date: Thu, 21 Dec 2023 15:10:55 +0000 (+0100)
Subject: strbuf: Rewrite `strbuf_print_restricted` using a bitmap.
X-Git-Tag: 6.0.0-rc0~28^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c6908182cedcc50b14d5fe4975a8b2d30fba5470;p=thirdparty%2Fcollectd.git

strbuf: Rewrite `strbuf_print_restricted` using a bitmap.

Instead of building the buffer piece by piece, copy the entire string into
the buffer and do the replacements there.

I think the code is quite efficient, but I haven't profiled either
version so can't say for sure that there is a speedup. The new code
*may* be easier to reason about, since the "copy and replace" approach
has a much simpler loop body than the previous approach.
---

diff --git a/src/utils/strbuf/strbuf.c b/src/utils/strbuf/strbuf.c
index 5dfce19df..d28f05fb0 100644
--- a/src/utils/strbuf/strbuf.c
+++ b/src/utils/strbuf/strbuf.c
@@ -290,6 +290,18 @@ int strbuf_print_escaped(strbuf_t *buf, char const *s, char const *need_escape,
   return 0;
 }
 
+static void bitmap_set(uint64_t b[static 4] , uint8_t v) {
+  uint8_t index = v / 64;
+  uint8_t bit = v % 64;
+  b[index] |= 1 << bit;
+}
+
+static bool bitmap_lookup(uint64_t b[static 4], uint8_t v) {
+  uint8_t index = v / 64;
+  uint8_t bit = v % 64;
+  return (b[index] & (1 << bit)) != 0;
+}
+
 int strbuf_print_restricted(strbuf_t *buf, char const *s, char const *accept,
                             char replace_char) {
   if (buf == NULL || s == NULL || accept == NULL || accept[0] == 0 ||
@@ -300,35 +312,28 @@ int strbuf_print_restricted(strbuf_t *buf, char const *s, char const *accept,
     return EINVAL;
   }
 
-  size_t s_len = strlen(s);
-  if (s_len == 0) {
-    return strbuf_print(buf, s);
+  /* Create a bitmap so we can do an efficient "accept" lookup without
+   * branching/looping. */
+  uint64_t bitmap[4] = {0};
+  for (size_t i = 0; accept[i] != 0; i++) {
+    bitmap_set(bitmap, (uint8_t)accept[i]);
   }
 
-  while (s_len > 0) {
-    size_t valid_len = strspn(s, accept);
-    if (valid_len == s_len) {
-      return strbuf_print(buf, s);
-    }
-    if (valid_len != 0) {
-      int status = strbuf_printn(buf, s, valid_len);
-      if (status != 0) {
-        return status;
-      }
+  size_t start_pos = buf->pos;
 
-      s += valid_len;
-      s_len -= valid_len;
-      continue;
-    }
+  int status = strbuf_print(buf, s);
+  if (status != 0) {
+    return status;
+  }
 
-    char tmp[] = {replace_char, 0};
-    int status = strbuf_print(buf, tmp);
-    if (status != 0) {
-      return status;
-    }
+  size_t s_len = strlen(s);
+  char conv[s_len + 1];
+  memcpy(conv, s, sizeof(conv));
 
-    s++;
-    s_len--;
+  for (size_t i = start_pos; buf->ptr[i] != 0; i++) {
+    if (!bitmap_lookup(bitmap, (uint8_t)buf->ptr[i])) {
+      buf->ptr[i] = replace_char;
+    }
   }
 
   return 0;