Refactor out the 'find string at start of any line' logic.

We do this in too many places throughout the code; it's time to start
clamping down.

Also, refactor Karsten's patch to use strchr-then-strndup, rather than
malloc-then-strlcpy-then-strchr-then-clear.

diff --git a/src/common/util.c b/src/common/util.c
index e70a9ea5f34b5acb67ca69f04d0b2f034ca500f3..6177a3e7368bbbf87aed784f817446dbee9510a9 100644 (file)
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -682,6 +682,29 @@ find_whitespace_eos(const char *s, const char *eos)
   return s;
 }
 
+/** Return the the first occurrence of <b>needle</b> in <b>haystack</b> that
+ * occurs at the start of a line (that is, at the beginning of <b>haystack</b>
+ * or immediately after a newline).  Return NULL if no such string is found.
+ */
+const char *
+find_str_at_start_of_line(const char *haystack, const char *needle)
+{
+  size_t needle_len = strlen(needle);
+
+  do {
+    if (!strncmp(haystack, needle, needle_len))
+      return haystack;
+
+    haystack = strchr(haystack, '\n');
+    if (!haystack)
+      return NULL;
+    else
+      ++haystack;
+  } while (*haystack);
+
+  return NULL;
+}
+
 /** Return true iff the 'len' bytes at 'mem' are all zero. */
 int
 tor_mem_is_zero(const char *mem, size_t len)

diff --git a/src/common/util.h b/src/common/util.h
index 17cbb4a44f196893f67c7ab7e71cac543f5359ac..b55bb91c51f9acfa650a530d68500779bfd436eb 100644 (file)
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -195,6 +195,8 @@ const char *eat_whitespace_no_nl(const char *s) ATTR_PURE;
 const char *eat_whitespace_eos_no_nl(const char *s, const char *eos) ATTR_PURE;
 const char *find_whitespace(const char *s) ATTR_PURE;
 const char *find_whitespace_eos(const char *s, const char *eos) ATTR_PURE;
+const char *find_str_at_start_of_line(const char *haystack, const char *needle)
+  ATTR_PURE;
 int tor_mem_is_zero(const char *mem, size_t len) ATTR_PURE;
 int tor_digest_is_zero(const char *digest) ATTR_PURE;
 int tor_digest256_is_zero(const char *digest) ATTR_PURE;

diff --git a/src/or/geoip.c b/src/or/geoip.c
index 535e9affff0195d393a0dabe9cc60b6129d56a72..d99e472118303b0b8182c4f8bb28d052436f3bbf 100644 (file)
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -1099,11 +1099,10 @@ parse_bridge_stats_controller(const char *stats_str, time_t now)
 {
   char stats_end_str[ISO_TIME_LEN+1], stats_start_str[ISO_TIME_LEN+1],
        *controller_str, *eos, *eol, *summary;
-  const char *stats_end_first = "bridge-stats-end ",
-             *stats_end_middle = "\nbridge-stats-end ",
-             *ips_first = "bridge-ips",
-             *ips_middle = "\nbridge-ips",
-             *tmp;
+
+  const char *BRIDGE_STATS_END = "bridge-stats-end ";
+  const char *BRIDGE_IPS = "bridge-ips ";
+  const char *tmp;
   time_t stats_end_time;
   size_t controller_len;
   int seconds;
@@ -1111,14 +1110,11 @@ parse_bridge_stats_controller(const char *stats_str, time_t now)
 
   /* Parse timestamp and number of seconds from
     "bridge-stats-end YYYY-MM-DD HH:MM:SS (N s)" */
-  if (!strncmp(stats_str, stats_end_first, strlen(stats_end_first))) {
-    tmp = stats_str + strlen(stats_end_first);
-  } else {
-    tmp = strstr(stats_str, stats_end_middle);
-    if (!tmp)
-      return NULL;
-    tmp += strlen(stats_end_middle);
-  }
+  tmp = find_str_at_start_of_line(stats_str, BRIDGE_STATS_END);
+  if (!tmp)
+    return NULL;
+  tmp += strlen(BRIDGE_STATS_END);
+
   if (strlen(tmp) < ISO_TIME_LEN + 6)
     return NULL;
   strlcpy(stats_end_str, tmp, sizeof(stats_end_str));
@@ -1133,23 +1129,19 @@ parse_bridge_stats_controller(const char *stats_str, time_t now)
   format_iso_time(stats_start_str, stats_end_time - seconds);
 
   /* Parse: "bridge-ips CC=N,CC=N,..." */
-  if (!strncmp(stats_str, ips_first, strlen(ips_first))) {
-    tmp = stats_str + strlen(ips_first);
-  } else {
-    tmp = strstr(stats_str, ips_middle);
-    if (!tmp)
-      return NULL;
-    tmp += strlen(ips_middle);
-  }
-  if (strlen(tmp) > 2 && !strncmp(tmp, " ", 1))
-    tmp += 1;
-  else
-    tmp = "";
-  summary = tor_malloc(strlen(tmp) + 1);
-  strlcpy(summary, tmp, strlen(tmp) + 1);
-  eol = strstr(summary, "\n");
+  tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS);
+  if (!tmp)
+    return NULL;
+  tmp += strlen(BRIDGE_IPS);
+
+  tmp = eat_whitespace_no_nl(tmp);
+
+  eol = strchr(tmp, '\n');
   if (eol)
-    eol[0] = '\0';
+    summary = tor_strndup(tmp, eol-tmp);
+  else
+    summary = tor_strdup(tmp);
+
   controller_len = strlen("TimeStarted=\"\" CountrySummary=") +
                           strlen(summary) + 42;
   controller_str = tor_malloc(controller_len);

diff --git a/src/test/test_util.c b/src/test/test_util.c
index 51b788e725720a04dff3534e9152f75655b692c3..3ed6a7aab8c71d404725c07526d823eb70039912 100644 (file)
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -1023,9 +1023,39 @@ test_util_strtok(void)
   ;
 }
 
+static void
+test_util_find_str_at_start_of_line(void *ptr)
+{
+  const char *long_string =
+    "hello world. hello world. hello hello. howdy.\n"
+    "hello hello world\n";
+
+  (void)ptr;
+
+  /* not-found case. */
+  tt_assert(! find_str_at_start_of_line(long_string, "fred"));
+
+  /* not-found case where haystack doesn't end with \n */
+  tt_assert(! find_str_at_start_of_line("foobar\nbaz", "fred"));
+
+  /* start-of-string case */
+  tt_assert(long_string ==
+            find_str_at_start_of_line(long_string, "hello world."));
+
+  /* start-of-line case */
+  tt_assert(strchr(long_string,'\n')+1 ==
+            find_str_at_start_of_line(long_string, "hello hello"));
+ done:
+  ;
+}
+
+
 #define UTIL_LEGACY(name)                                               \
   { #name, legacy_test_helper, 0, &legacy_setup, test_util_ ## name }
 
+#define UTIL_TEST(name, flags)                          \
+  { #name, test_util_ ## name, flags, NULL, NULL }
+
 struct testcase_t util_tests[] = {
   UTIL_LEGACY(time),
   UTIL_LEGACY(config_line),
@@ -1040,6 +1070,7 @@ struct testcase_t util_tests[] = {
   UTIL_LEGACY(threads),
   UTIL_LEGACY(sscanf),
   UTIL_LEGACY(strtok),
+  UTIL_TEST(find_str_at_start_of_line, 0),
   END_OF_TESTCASES
 };