Merge pull request #35 from lyft/validate-const

validate: don't modify input and remove alloc

src/tests/test_validate.c

@@ -6,17 +6,52 @@
 #include "../log.h"
 #include "../validate.h"
 
-
-int main(int argc, char **argv) {
-    stats_log_verbose(1);
-
+void test_validate_stat() {
     validate_parsed_result_t result;
 
     static const char* exp1 = "a.b.c.__tag1=v1.__tag2=v2:v2:42.000|ms";
 
     assert(0 == validate_statsd(exp1, strlen(exp1), &result));
     assert(42.0 == result.value);
     assert(METRIC_TIMER == result.type);
+}
+
+void test_parse_presampling_value() {
+    validate_parsed_result_t result;
+
+    static const char* exp1 = "test.srv.req:2.5|ms|@0.2";
+
+    assert(0 == validate_statsd(exp1, strlen(exp1), &result));
+    assert(2.5 == result.value);
+    assert(0.2 == result.presampling_value);
+    assert(METRIC_TIMER == result.type);
+}
+
+void test_line_not_modified() {
+    const char *lines[] = {
+        "a.b.c.__tag1=v1.__tag2=v2:v2:42.000|ms",
+        "test.srv.req:2.5|ms|@0.2",
+    };
+    const int lines_len = sizeof(lines) / sizeof(char *);
+
+    for (int i = 0; i < lines_len; i++) {
+        const char *line = lines[i];
+        const char *copy = strdup(line);
+
+        validate_parsed_result_t result;
+        assert(validate_statsd(line, strlen(line), &result) == 0);
+        assert(strcmp(line, copy) == 0);
+    }
+}
+
+int main() {
+    stats_log_verbose(1);
+
+    test_validate_stat();
+
+    test_parse_presampling_value();
+
+    test_line_not_modified();
 
     return 0;
 }

src/validate.c

@@ -4,125 +4,102 @@
 
 #include <string.h>
 
-static char *valid_stat_types[6] = {
+static const char * const valid_stat_types[6] = {
     "c",
     "ms",
     "kv",
     "g",
     "h",
     "s"
 };
-static size_t valid_stat_types_len = 6;
+static const int valid_stat_types_len = 6;
 
 // For some reason this is not in string.h, probably because it was introduced
 // there by a GNU extention:
 // "The memrchr() function  is  a  GNU  extension,  available  since  glibc 2.1.91."
 // (http://manpages.ubuntu.com/manpages/xenial/man3/memchr.3.html)
 extern void *memrchr(const void*, int, size_t);
 
-int validate_statsd(const char *line, size_t len, validate_parsed_result_t* result) {
-    size_t plen;
-    char c;
-    int i, valid;
-
-    // FIXME: this is dumb, don't do a memory copy
-    char *line_copy = strndup(line, len);
-    char *start, *end;
-    char *err;
-
-    if (line_copy == NULL) {
-        stats_log("validate: allocation failure. marking invalid line");
-        goto statsd_err;
+static metric_type parse_stat_type(const char *str, size_t len) {
+    if (1 <= len && len <= 2) {
+        for (int i = 0; i < valid_stat_types_len; i++) {
+            const char *stat = valid_stat_types[i];
+            // len is bounded, so help the the compiler optimize
+            if ((len == 1 ? memcmp(stat, str, 1) : memcmp(stat, str, 2)) == 0) {
+                return (metric_type)i;
+            }
+        }
     }
+    return METRIC_UNKNOWN;
+}
+
+int validate_statsd(const char *line, size_t len, validate_parsed_result_t* result) {
+    const char *start = line;
+    size_t plen = len;
 
-    start = line_copy;
-    plen = len;
     // Search backwards, otherwise might eat up irrelevant data.
     // Example: keyname.__tagname=tag:value:42.0|ms
     //                                      ^^^^--- actual value
-    end = memrchr(start, ':', plen);
+    const char *end = memrchr(start, ':', plen);
     if (end == NULL) {
         stats_log("validate: Invalid line \"%.*s\" missing ':'", len, line);
-        goto statsd_err;
+        return 1;
     }
-
     if ((end - start) < 1) {
         stats_log("validate: Invalid line \"%.*s\" zero length key", len, line);
-        goto statsd_err;
+        return 1;
     }
-
     start = end + 1;
-    plen = len - (start - line_copy);
+    plen = len - (start - line);
 
-    c = end[0];
-    end[0] = '\0';
-    result->presampling_value = 1.0; /* Default pre-sampling to 1.0 */
+    char *err;
     result->value = strtod(start, &err);
-    if ((result->value == 0.0) && (err == start)) {
+    if (result->value == 0 && err == start) {
         stats_log("validate: Invalid line \"%.*s\" unable to parse value as double", len, line);
-        goto statsd_err;
+        return 1;
     }
-    end[0] = c;
 
     end = memchr(start, '|', plen);
     if (end == NULL) {
         stats_log("validate: Invalid line \"%.*s\" missing '|'", len, line);
-        goto statsd_err;
+        return 1;
     }
-
     start = end + 1;
-    plen = len - (start - line_copy);
+    plen = len - (start - line);
 
     end = memchr(start, '|', plen);
     if (end != NULL) {
-        c = end[0];
-        end[0] = '\0';
         plen = end - start;
     }
 
-    valid = 0;
-    for (i = 0; i < valid_stat_types_len; i++) {
-        if (strlen(valid_stat_types[i]) != plen) {
-            continue;
-        }
-        if (strncmp(start, valid_stat_types[i], plen) == 0) {
-            result->type = (metric_type)i; /* The indexes match the enum values in the comparison list */
-            valid = 1;
-            break;
-        }
-    }
-
-    if (valid == 0) {
+    result->type = parse_stat_type(start, plen);
+    if (result->type < 0) {
         stats_log("validate: Invalid line \"%.*s\" unknown stat type \"%.*s\"", len, line, plen, start);
-        goto statsd_err;
+        return 1;
     }
 
+    result->presampling_value = 1.0; /* Default pre-sampling to 1.0 */
+
     if (end != NULL) {
-        end[0] = c;
         // end[0] is currently the second | char
         // test if we have at least 1 char following it (@)
-        if ((len - (end - line_copy) > 1) && (end[1] == '@')) {
+        if ((len - (end - line) > 1) && (end[1] == '@')) {
             start = end + 2;
-            plen = len - (start - line_copy);
+            plen = len - (start - line);
             if (plen == 0) {
                 stats_log("validate: Invalid line \"%.*s\" @ sample with no rate", len, line);
-                goto statsd_err;
+                return 1;
             }
             result->presampling_value = strtod(start, &err);
             if ((result->presampling_value == 0.0) && err == start) {
                 stats_log("validate: Invalid line \"%.*s\" invalid sample rate", len, line);
-                goto statsd_err;
+                return 1;
             }
         } else {
             stats_log("validate: Invalid line \"%.*s\" no @ sample rate specifier", len, line);
-            goto statsd_err;
+            return 1;
         }
     }
 
-    free(line_copy);
     return 0;
-
-statsd_err:
-    free(line_copy);
-    return 1;
 }