Merge branch 'config-no-null-strings-in-printf'

config.c

@@ -209,10 +209,19 @@ log_contextual(struct context *ctx, enum log_class log_class,
     char *formatted_msg = xvasprintf(fmt, va);
     va_end(va);
 
+    bool print_dot = ctx->key != NULL;
+    bool print_colon = ctx->value != NULL;
+
+    if (!print_dot)
+        ctx->key = "";
+
+    if (!print_colon)
+        ctx->value = "";
+
     log_and_notify(
-        ctx->conf, log_class, file, lineno, "%s:%d: [%s].%s: %s: %s",
+        ctx->conf, log_class, file, lineno, "%s:%d: [%s]%s%s%s%s: %s",
-        ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value,
+        ctx->path, ctx->lineno, ctx->section, print_dot ? "." : "",
-        formatted_msg);
+        ctx->key, print_colon ? ": " : "", ctx->value, formatted_msg);
     free(formatted_msg);
 }
 
@@ -744,7 +753,7 @@ value_to_fonts(struct context *ctx)
          font = strtok(NULL, ","))
     {
         /* Trim spaces, strictly speaking not necessary, but looks nice :) */
-        while (*font != '\0' && isspace(*font))
+        while (isspace(font[0]))
             font++;
 
         if (font[0] == '\0')
@@ -1153,13 +1162,14 @@ parse_section_url(struct context *ctx)
         {
 
             /* Strip leading whitespace */
-            while (isspace(*prot))
+            while (isspace(prot[0]))
                 prot++;
 
             /* Strip trailing whitespace */
             size_t len = strlen(prot);
-            while (len > 0 && isspace(prot[len - 1]))
+            while (isspace(prot[len - 1]))
-                prot[--len] = '\0';
+                len--;
+            prot[len] = '\0';
 
             size_t chars = mbstowcs(NULL, prot, 0);
             if (chars == (size_t)-1) {
@@ -2321,56 +2331,63 @@ parse_section_tweak(struct context *ctx)
 static bool
 parse_key_value(char *kv, const char **section, const char **key, const char **value)
 {
-    /*strip leading whitespace*/
+    bool section_is_needed = section != NULL;
-    while (*kv && isspace(*kv))
+
+    /* Strip leading whitespace */
+    while (isspace(kv[0]))
         ++kv;
 
-    if (section != NULL)
+    if (section_is_needed)
-        *section = NULL;
+        *section = "main";
+
+    if (kv[0] == '=')
+        return false;
+
     *key = kv;
     *value = NULL;
 
     size_t kvlen = strlen(kv);
+
+    /* Strip trailing whitespace */
+    while (isspace(kv[kvlen - 1]))
+        kvlen--;
+    kv[kvlen] = '\0';
+
     for (size_t i = 0; i < kvlen; ++i) {
-        if (kv[i] == '.') {
+        if (kv[i] == '.' && section_is_needed) {
-            if (section != NULL && *section == NULL) {
+            section_is_needed = false;
-                *section = kv;
+            *section = kv;
-                kv[i] = '\0';
-                *key = &kv[i + 1];
-            }
-        } else if (kv[i] == '=') {
-            if (section != NULL && *section == NULL)
-                *section = "main";
             kv[i] = '\0';
-            *value = &kv[i + 1];
+            if (i == kvlen - 1 || kv[i + 1] == '=') {
+                *key = NULL;
+                return false;
+            }
+            *key = &kv[i + 1];
+        } else if (kv[i] == '=') {
+            kv[i] = '\0';
+            if (i != kvlen - 1)
+                *value = &kv[i + 1];
             break;
         }
     }
+
     if (*value == NULL)
         return false;
 
     /* Strip trailing whitespace from key (leading stripped earlier) */
     {
-        xassert(!isspace(**key));
+        xassert(!isspace(*key[0]));
 
         char *end = (char *)*key + strlen(*key) - 1;
-        while (isspace(*end))
+        while (isspace(end[0]))
             end--;
-        *(end + 1) = '\0';
+        end[1] = '\0';
     }
 
-    /* Strip leading+trailing whitespace from valueue */
+    /* Strip leading whitespace from value (trailing stripped earlier) */
-    {
+    while (isspace(*value[0]))
-        while (isspace(**value))
+        ++*value;
-            ++*value;
 
-        if (*value[0] != '\0') {
-            char *end = (char *)*value + strlen(*value) - 1;
-            while (isspace(*end))
-                end--;
-            *(end + 1) = '\0';
-        }
-    }
     return true;
 }
 
@@ -2432,12 +2449,14 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar
 
     char *_line = NULL;
     size_t count = 0;
+    bool ret = true;
 
 #define error_or_continue()                     \
     {                                           \
-        if (errors_are_fatal)                   \
+        if (errors_are_fatal) {                 \
-            goto err;                           \
+            ret = false;                        \
-        else                                    \
+            goto done;                          \
+        } else                                  \
             continue;                           \
     }
 
@@ -2452,69 +2471,90 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar
     };
     struct context *ctx = &context;  /* For LOG_AND_*() */
 
-    while (true) {
+    errno = 0;
-        errno = 0;
+    ssize_t len;
+
+    while ((len = getline(&_line, &count, f)) != -1) {
+        context.key = NULL;
+        context.value = NULL;
         context.lineno++;
 
-        ssize_t ret = getline(&_line, &count, f);
+        char *line = _line;
-
-        if (ret < 0) {
-            if (errno != 0) {
-                LOG_AND_NOTIFY_ERRNO("failed to read from configuration");
-                if (errors_are_fatal)
-                    goto err;
-            }
-            break;
-        }
 
         /* Strip leading whitespace */
-        char *line = _line;
+        while (isspace(line[0])) {
-        {
+            line++;
-            while (isspace(*line))
+            len--;
-                line++;
-            if (line[0] != '\0') {
-                char *end = line + strlen(line) - 1;
-                while (isspace(*end))
-                    end--;
-                *(end + 1) = '\0';
-            }
         }
 
         /* Empty line, or comment */
         if (line[0] == '\0' || line[0] == '#')
             continue;
 
+        /* Strip the trailing newline - may be absent on the last line */
+        if (line[len - 1] == '\n')
+            line[--len] = '\0';
+
         /* Split up into key/value pair + trailing comment separated by blank */
         char *key_value = line;
-        char *comment = line;
+        char *kv_trailing = &line[len - 1];
-        while (comment[0] != '\0') {
+        char *comment = &line[1];
-            const char c = comment[0];
+        while (comment[1] != '\0') {
-            comment++;
+            if (isblank(comment[0]) && comment[1] == '#') {
-            if (isblank(c) && comment[0] == '#') {
+                comment[1] = '\0'; /* Terminate key/value pair */
-                comment[0] = '\0'; /* Terminate key/value pair */
+                kv_trailing = comment++;
-                comment++;
                 break;
             }
+            comment++;
         }
+        comment++;
+
+        /* Strip trailing whitespace */
+        while (isspace(kv_trailing[0]))
+            kv_trailing--;
+        kv_trailing[1] = '\0';
 
         /* Check for new section */
         if (key_value[0] == '[') {
-            char *end = strchr(key_value, ']');
+            key_value++;
-            if (end == NULL) {
+
-                LOG_CONTEXTUAL_ERR("syntax error: no closing ']'");
+            if (key_value[0] == ']') {
+                LOG_CONTEXTUAL_ERR("empty section name");
+                section = SECTION_COUNT;
                 error_or_continue();
             }
 
-            *end = '\0';
+            char *end = strchr(key_value, ']');
 
-            section = str_to_section(&key_value[1]);
+            if (end == NULL) {
+                context.section = key_value;
+                LOG_CONTEXTUAL_ERR("syntax error: no closing ']'");
+                context.section = section_name;
+                section = SECTION_COUNT;
+                error_or_continue();
+            }
+
+            end[0] = '\0';
+
+            if (end[1] != '\0') {
+                context.section = key_value;
+                LOG_CONTEXTUAL_ERR("section declaration contains trailing "
+                                   "characters");
+                context.section = section_name;
+                section = SECTION_COUNT;
+                error_or_continue();
+            }
+
+            section = str_to_section(key_value);
             if (section == SECTION_COUNT) {
-                LOG_CONTEXTUAL_ERR("invalid section name: %s", &key_value[1]);
+                context.section = key_value;
+                LOG_CONTEXTUAL_ERR("invalid section name: %s", key_value);
+                context.section = section_name;
                 error_or_continue();
             }
 
             free(section_name);
-            section_name = xstrdup(&key_value[1]);
+            section_name = xstrdup(key_value);
             context.section = section_name;
 
             /* Process next line */
@@ -2527,14 +2567,13 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar
         }
 
         if (!parse_key_value(key_value, NULL, &context.key, &context.value)) {
-            LOG_CONTEXTUAL_ERR("syntax error: key/value pair has no value");
+            LOG_CONTEXTUAL_ERR("syntax error: key/value pair has no %s",
-            if (errors_are_fatal)
+                               context.key == NULL ? "key" : "value");
-                goto err;
+            error_or_continue();
-            break;
         }
 
         LOG_DBG("section=%s, key='%s', value='%s', comment='%s'",
-                section_info[section].name, key, value, comment);
+                section_info[section].name, context.key, context.value, comment);
 
         xassert(section >= 0 && section < SECTION_COUNT);
 
@@ -2545,14 +2584,16 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar
             error_or_continue();
     }
 
-    free(section_name);
+    if (errno != 0) {
-    free(_line);
+        LOG_AND_NOTIFY_ERRNO("failed to read from configuration");
-    return true;
+        if (errors_are_fatal)
+            ret = false;
+    }
 
-err:
+done:
     free(section_name);
     free(_line);
-    return false;
+    return ret;
 }
 
 static char *
@@ -2977,7 +3018,15 @@ config_override_apply(struct config *conf, config_override_t *overrides,
         if (!parse_key_value(
                 it->item, &context.section, &context.key, &context.value))
         {
-            LOG_CONTEXTUAL_ERR("syntax error: key/value pair has no value");
+            LOG_CONTEXTUAL_ERR("syntax error: key/value pair has no %s",
+                               context.key == NULL ? "key" : "value");
+            if (errors_are_fatal)
+                return false;
+            continue;
+        }
+
+        if (context.section[0] == '\0') {
+            LOG_CONTEXTUAL_ERR("empty section name");
             if (errors_are_fatal)
                 return false;
             continue;