From 65b5469e86dc57969cac2109ce5bd53a88164b08 Mon Sep 17 00:00:00 2001 From: Pranjal Kole Date: Sat, 29 Jan 2022 23:10:26 +0530 Subject: [PATCH 1/3] config: don't pass null strings to *printf() musl and glibc's *printf() convert NULL strings to "(null)" [0][1], but this is undefined behaviour. context.value has to be set to two backspaces, so that the extra colon is removed. context.key is set to one backspace, so that the extra dot is removed. context.section is now set to "main" by default, so it is never NULL. [0]: https://git.musl-libc.org/cgit/musl/tree/src/stdio/vfprintf.c#n593 [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/vfprintf-internal.c;hb=HEAD#l1011 --- config.c | 83 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 20 deletions(-) diff --git a/config.c b/config.c index 9004630d..11dcc5cf 100644 --- a/config.c +++ b/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->path, ctx->lineno, ctx->section, ctx->key, ctx->value, - formatted_msg); + ctx->conf, log_class, file, lineno, "%s:%d: [%s]%s%s%s%s: %s", + ctx->path, ctx->lineno, ctx->section, print_dot ? "." : "", + ctx->key, print_colon ? ": " : "", ctx->value, formatted_msg); free(formatted_msg); } @@ -2321,31 +2330,41 @@ parse_section_tweak(struct context *ctx) static bool parse_key_value(char *kv, const char **section, const char **key, const char **value) { + bool section_is_needed = section != NULL; + /*strip leading whitespace*/ while (*kv && isspace(*kv)) ++kv; - if (section != NULL) - *section = NULL; + if (section_is_needed) + *section = "main"; + + if (kv[0] == '=') + return false; + *key = kv; *value = NULL; size_t kvlen = strlen(kv); for (size_t i = 0; i < kvlen; ++i) { - if (kv[i] == '.') { - if (section != NULL && *section == NULL) { - *section = kv; - kv[i] = '\0'; - *key = &kv[i + 1]; - } - } else if (kv[i] == '=') { - if (section != NULL && *section == NULL) - *section = "main"; + if (kv[i] == '.' && section_is_needed) { + section_is_needed = false; + *section = kv; kv[i] = '\0'; + 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) + return false; *value = &kv[i + 1]; break; } } + if (*value == NULL) return false; @@ -2499,22 +2518,37 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar /* Check for new section */ if (key_value[0] == '[') { + key_value++; + + if (key_value[0] == ']') { + LOG_CONTEXTUAL_ERR("empty section name"); + error_or_continue(); + } + + context.section = key_value; char *end = strchr(key_value, ']'); + if (end == NULL) { LOG_CONTEXTUAL_ERR("syntax error: no closing ']'"); error_or_continue(); } - *end = '\0'; + end[0] = '\0'; - section = str_to_section(&key_value[1]); + if (end[1] != '\0') { + LOG_CONTEXTUAL_ERR("section declaration contains trailing " + "characters"); + error_or_continue(); + } + + section = str_to_section(key_value); if (section == SECTION_COUNT) { - LOG_CONTEXTUAL_ERR("invalid section name: %s", &key_value[1]); + LOG_CONTEXTUAL_ERR("invalid section name: %s", key_value); 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,7 +2561,8 @@ 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", + context.key == NULL ? "key" : "value"); if (errors_are_fatal) goto err; break; @@ -2977,7 +3012,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; From 06fdebbbcbc47562e4703a9c9ac3c2c16bb545c5 Mon Sep 17 00:00:00 2001 From: Pranjal Kole Date: Fri, 4 Feb 2022 18:30:29 +0530 Subject: [PATCH 2/3] config: use getline idiomatically getline(3) contains an example program at the end, showing its usage. A few other changes have also been made. --- config.c | 138 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 72 insertions(+), 66 deletions(-) diff --git a/config.c b/config.c index 11dcc5cf..7da56345 100644 --- a/config.c +++ b/config.c @@ -724,7 +724,7 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) long value = strtol(s, &end, 10); if (!(errno == 0 && end == s + len - 2)) { - LOG_CONTEXTUAL_ERR("invalid px value (must be on the form 12px)"); + LOG_CONTEXTUAL_ERR("invalid px value (must be of the form 12px)"); return false; } res->pt = 0; @@ -753,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') @@ -1162,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])) - prot[--len] = '\0'; + while (isspace(prot[len - 1])) + len--; + prot[len] = '\0'; size_t chars = mbstowcs(NULL, prot, 0); if (chars == (size_t)-1) { @@ -2332,8 +2333,8 @@ parse_key_value(char *kv, const char **section, const char **key, const char **v { bool section_is_needed = section != NULL; - /*strip leading whitespace*/ - while (*kv && isspace(*kv)) + /* Strip leading whitespace */ + while (isspace(kv[0])) ++kv; if (section_is_needed) @@ -2346,6 +2347,12 @@ parse_key_value(char *kv, const char **section, const char **key, const char **v *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] == '.' && section_is_needed) { section_is_needed = false; @@ -2358,9 +2365,8 @@ parse_key_value(char *kv, const char **section, const char **key, const char **v *key = &kv[i + 1]; } else if (kv[i] == '=') { kv[i] = '\0'; - if (i == kvlen - 1) - return false; - *value = &kv[i + 1]; + if (i != kvlen - 1) + *value = &kv[i + 1]; break; } } @@ -2370,26 +2376,18 @@ parse_key_value(char *kv, const char **section, const char **key, const char **v /* 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 */ - { - while (isspace(**value)) - ++*value; + /* Strip leading whitespace from value (trailing stripped earlier) */ + while (isspace(*value[0])) + ++*value; - if (*value[0] != '\0') { - char *end = (char *)*value + strlen(*value) - 1; - while (isspace(*end)) - end--; - *(end + 1) = '\0'; - } - } return true; } @@ -2451,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) \ - goto err; \ - else \ + if (errors_are_fatal) { \ + ret = false; \ + goto done; \ + } else \ continue; \ } @@ -2471,50 +2471,48 @@ 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); - - if (ret < 0) { - if (errno != 0) { - LOG_AND_NOTIFY_ERRNO("failed to read from configuration"); - if (errors_are_fatal) - goto err; - } - break; - } + char *line = _line; /* Strip leading whitespace */ - char *line = _line; - { - while (isspace(*line)) - line++; - if (line[0] != '\0') { - char *end = line + strlen(line) - 1; - while (isspace(*end)) - end--; - *(end + 1) = '\0'; - } + while (isspace(line[0])) { + line++; + len--; } /* 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; - while (comment[0] != '\0') { - const char c = comment[0]; - comment++; - if (isblank(c) && comment[0] == '#') { - comment[0] = '\0'; /* Terminate key/value pair */ - comment++; + char *kv_trailing = &line[len - 1]; + char *comment = &line[1]; + while (comment[1] != '\0') { + if (isblank(comment[0]) && comment[1] == '#') { + comment[1] = '\0'; /* Terminate key/value pair */ + kv_trailing = 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] == '[') { @@ -2522,28 +2520,36 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar if (key_value[0] == ']') { LOG_CONTEXTUAL_ERR("empty section name"); + section = SECTION_COUNT; error_or_continue(); } - context.section = key_value; char *end = strchr(key_value, ']'); 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) { + context.section = key_value; LOG_CONTEXTUAL_ERR("invalid section name: %s", key_value); + context.section = section_name; error_or_continue(); } @@ -2563,13 +2569,11 @@ 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 %s", context.key == NULL ? "key" : "value"); - if (errors_are_fatal) - goto err; - break; + error_or_continue(); } 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); @@ -2580,14 +2584,16 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar error_or_continue(); } - free(section_name); - free(_line); - return true; + if (errno != 0) { + LOG_AND_NOTIFY_ERRNO("failed to read from configuration"); + if (errors_are_fatal) + ret = false; + } -err: +done: free(section_name); free(_line); - return false; + return ret; } static char * From 343f2c51a4cc44aa5ae3a813ae1c908b2f01b9f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 5 Feb 2022 16:59:42 +0100 Subject: [PATCH 3/3] config: s/of the form/on the form/ --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index 7da56345..18f3a5df 100644 --- a/config.c +++ b/config.c @@ -724,7 +724,7 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) long value = strtol(s, &end, 10); if (!(errno == 0 && end == s + len - 2)) { - LOG_CONTEXTUAL_ERR("invalid px value (must be of the form 12px)"); + LOG_CONTEXTUAL_ERR("invalid px value (must be on the form 12px)"); return false; } res->pt = 0;