[ovs-dev] [PATCH v3 2/7] ovs-ofctl: declare / set up colors for command output

[Ben Pfaff]

On Wed, Mar 02, 2016 at 03:56:17PM +0100, Quentin Monnet wrote:
> OVS_COLORS environment variable is parsed to extract user-defined
> preferences regarding colors (this is used to set up a color theme, not
> to replace the `--color` option for activating color output).
> 
> The string should be of a format similar to LS_COLORS or GREP_COLORS,
> with available colors being as follows:
> 
> * ac: action field
> * dr: drop keyword
> * le: learn keyword
> * pm: parameters receiving attributes
> * pr: keyword having parenthesis
> * sp: some special keywords
> * vl: lone values with no parameter name
> 
> For color whose idendifier does not appear in the string, the default
> hardcoded value is used instead.
> 
> As an example, setting OVS_COLORS to the following string is equivalent
> to using the default values:
> 
>     OVS_COLORS="ac:01;31:dr=34:le=31:pm=36:pr=35:sp=33:vl=32"
> 
> Signed-off-by: Quentin Monnet <quentin.monnet at 6wind.com>

Thanks.  I'm going to fold in the following style fixes:

diff --git a/lib/colors.c b/lib/colors.c
index 2ed4bad..4ea1880 100644
--- a/lib/colors.c
+++ b/lib/colors.c
@@ -64,15 +64,14 @@ colors_init(bool enable_color)
     /* Color IDs to use in OVS_COLORS environment variable to overwrite
      * defaults with custom colors.
      */
-    const struct color_key color_dic[] =
-    {
-        { "ac", &(colors.actions) },
-        { "dr", &(colors.drop) },
-        { "le", &(colors.learn) },
-        { "pm", &(colors.param) },
-        { "pr", &(colors.paren) },
-        { "sp", &(colors.special) },
-        { "vl", &(colors.value) },
+    const struct color_key color_dic[] = {
+        { "ac", &colors.actions },
+        { "dr", &colors.drop },
+        { "le", &colors.learn },
+        { "pm", &colors.param },
+        { "pr", &colors.paren },
+        { "sp", &colors.special },
+        { "vl", &colors.value },
         { NULL, NULL }
     };
 
@@ -106,25 +105,19 @@ colors_init(bool enable_color)
 static void
 colors_parse_from_env(const struct color_key color_dic[])
 {
-    const char *color_str;
-    char *s;
-    char *token;
-
-    color_str = getenv("OVS_COLORS");
-    if (color_str == NULL || *color_str == '\0')
-    {
+    const char *color_str = getenv("OVS_COLORS");
+    if (color_str == NULL || *color_str == '\0') {
         return;
     }
-    s = xstrdup(color_str);
 
     /* Loop on tokens: they are separated by columns ':' */
-    for (token = strsep(&s, ":");
-            token != NULL;
-            token = strsep(&s, ":")) {
-        char *ptr;
+    char *s = xstrdup(color_str);
+    for (char *token = strsep(&s, ":");
+         token != NULL;
+         token = strsep(&s, ":")) {
         char *name = strsep(&token, "=");
-        for (ptr = token; ptr != NULL && *ptr != '\0'; ptr++) {
-            /* We accept only decimals and ';' for color marker */
+        for (char *ptr = token; ptr != NULL && *ptr != '\0'; ptr++) {
+            /* We accept only decimals and ';' for color marker. */
             if (*ptr == ';' || (*ptr >= '0' && *ptr <= '9')) {
                 continue;
             }
@@ -132,15 +125,14 @@ colors_parse_from_env(const struct color_key color_dic[])
             break;
         }
         if (name != NULL) {
-            /* We found a name and marker contains only decimals and ';'
-             * Try to get a pointer to associated color variable
-             */
+            /* We found a name and marker contains only decimals and ';'.
+             * Try to get a pointer to associated color variable. */
             char **color_var_ptr = get_color(color_dic, name);
-            /* If we know that color, update its value */
+            /* If we know that color, update its value. */
             if (color_var_ptr != NULL) {
-                *color_var_ptr = malloc(sizeof(char)*(11 + strlen(token)));
-                sprintf(*color_var_ptr, "\33[%sm\33[K", token);
+                *color_var_ptr = xasprintf("\33[%sm\33[K", token);
             }
         }
     }
+    free(s);
 }