color: refactor parse_attr

The list of attributes we recognize is a bit unwieldy, as we
actually have two arrays that must be kept in sync. Instead,
let's have a single array-of-struct to represent our
mapping. That means we can never have an accident that
causes us to read off the end of an array, and it makes
diffs for adding new attributes much easier to read.

This also makes it easy to handle the "no" cases without
having to repeat each attribute (this shortens the list,
making it easier to read, but also also cuts the size of our
linear search in half). Technically this makes it impossible
for us to add an attribute that starts with "no" (we could
confuse "nobody" for the negation of "body"), but since this
is a constrained set of attributes, that's OK.

Since we can also store the length of each name in the
struct, that makes it easy for us to avoid reading past the
"len" parameter given to us (though in practice it was not a
bug, since all of our current callers are interested in a
subset of a NUL-terminated buffer, not a true undelimited
range of memory).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/color.c b/color.c
index 9027352ad7786746537adea9007bd46f46edbb4e..63e7b0c9a1db05aadeda8e920be06a9e3822dc46 100644 (file)
--- a/color.c
+++ b/color.c
@@ -123,19 +123,30 @@ static int parse_color(struct color *out, const char *name, int len)
        return -1;
 }
 
        return -1;
 }
 

-static int parse_attr(const char *name, int len)
+static int parse_attr(const char *name, size_t len)

 {
 {

-       static const int attr_values[] = { 1, 2, 4, 5, 7,
-                                          22, 22, 24, 25, 27 };
-       static const char * const attr_names[] = {
-               "bold", "dim", "ul", "blink", "reverse",
-               "nobold", "nodim", "noul", "noblink", "noreverse"
+       static const struct {
+               const char *name;
+               size_t len;
+               int val, neg;
+       } attrs[] = {
+#define ATTR(x, val, neg) { (x), sizeof(x)-1, (val), (neg) }
+               ATTR("bold",      1, 22),
+               ATTR("dim",       2, 22),
+               ATTR("ul",        4, 24),
+               ATTR("blink",     5, 25),
+               ATTR("reverse",   7, 27)
+#undef ATTR

        };
        };

+       int negate = 0;

        int i;
        int i;

-       for (i = 0; i < ARRAY_SIZE(attr_names); i++) {
-               const char *str = attr_names[i];
-               if (!strncasecmp(name, str, len) && !str[len])
-                       return attr_values[i];
+
+       if (skip_prefix_mem(name, len, "no", &name, &len))
+               negate = 1;
+
+       for (i = 0; i < ARRAY_SIZE(attrs); i++) {
+               if (attrs[i].len == len && !memcmp(attrs[i].name, name, len))
+                       return negate ? attrs[i].neg : attrs[i].val;

        }
        return -1;
 }
        }
        return -1;
 }