From 82a788532417d5e297bc95a7f2634b9574b3d346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 13:50:14 +0100 Subject: [PATCH 01/23] config: pass aggregated struct to top-level section handlers --- config.c | 191 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 138 insertions(+), 53 deletions(-) diff --git a/config.c b/config.c index 91dfb871..5f199721 100644 --- a/config.c +++ b/config.c @@ -121,6 +121,18 @@ static const char *const binding_action_map[] = { static_assert(ALEN(binding_action_map) == BIND_ACTION_COUNT, "binding action map size mismatch"); +struct context { + struct config *conf; + const char *section; + const char *key; + const char *value; + + const char *path; + unsigned lineno; + + bool errors_are_fatal; +}; + static void NOINLINE PRINTF(5) log_and_notify(struct config *conf, enum log_class log_class, const char *file, int lineno, const char *fmt, ...) @@ -679,9 +691,15 @@ static bool parse_config_file( FILE *f, struct config *conf, const char *path, bool errors_are_fatal); static bool -parse_section_main(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_main(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + bool errors_are_fatal = ctx->errors_are_fatal; + if (strcmp(key, "include") == 0) { char *_include_path = NULL; const char *include_path = NULL; @@ -1049,9 +1067,14 @@ parse_section_main(const char *key, const char *value, struct config *conf, } static bool -parse_section_bell(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_bell(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "urgent") == 0) conf->bell.urgent = str_to_bool(value); else if (strcmp(key, "notify") == 0) @@ -1072,9 +1095,14 @@ parse_section_bell(const char *key, const char *value, struct config *conf, } static bool -parse_section_scrollback(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_scrollback(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "lines") == 0) { unsigned long lines; if (!str_to_ulong(value, 10, &lines)) { @@ -1148,9 +1176,14 @@ parse_section_scrollback(const char *key, const char *value, struct config *conf } static bool -parse_section_url(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_url(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "launch") == 0) { if (!str_to_spawn_template(conf, value, &conf->url.launch, path, lineno, "url", "launch")) @@ -1260,9 +1293,14 @@ parse_section_url(const char *key, const char *value, struct config *conf, } static bool -parse_section_colors(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_colors(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + size_t key_len = strlen(key); uint8_t last_digit = (unsigned char)key[key_len - 1] - '0'; uint32_t *color = NULL; @@ -1358,9 +1396,14 @@ parse_section_colors(const char *key, const char *value, struct config *conf, } static bool -parse_section_cursor(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_cursor(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "style") == 0) { if (strcmp(value, "block") == 0) conf->cursor.style = CURSOR_BLOCK; @@ -1416,9 +1459,14 @@ parse_section_cursor(const char *key, const char *value, struct config *conf, } static bool -parse_section_mouse(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_mouse(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "hide-when-typing") == 0) conf->mouse.hide_when_typing = str_to_bool(value); @@ -1435,9 +1483,14 @@ parse_section_mouse(const char *key, const char *value, struct config *conf, } static bool -parse_section_csd(const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_csd(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "preferred") == 0) { if (strcmp(value, "server") == 0) conf->csd.preferred = CONF_CSD_PREFER_SERVER; @@ -2029,20 +2082,28 @@ UNITTEST } static bool -parse_section_key_bindings( - const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_key_bindings(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + return parse_key_binding_section( "key-bindings", key, value, BIND_ACTION_KEY_COUNT, binding_action_map, &conf->bindings.key, conf, path, lineno); } static bool -parse_section_search_bindings( - const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_search_bindings(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + static const char *const search_binding_action_map[] = { [BIND_ACTION_SEARCH_NONE] = NULL, [BIND_ACTION_SEARCH_CANCEL] = "cancel", @@ -2074,10 +2135,14 @@ parse_section_search_bindings( } static bool -parse_section_url_bindings( - const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_url_bindings(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + static const char *const url_binding_action_map[] = { [BIND_ACTION_URL_NONE] = NULL, [BIND_ACTION_URL_CANCEL] = "cancel", @@ -2249,10 +2314,14 @@ has_mouse_binding_collisions(struct config *conf, const struct key_combo_list *k static bool -parse_section_mouse_bindings( - const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_mouse_bindings(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + char **pipe_argv; ssize_t pipe_remove_len = pipe_argv_from_string( @@ -2352,10 +2421,14 @@ parse_section_mouse_bindings( } static bool -parse_section_tweak( - const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal) +parse_section_tweak(struct context *ctx) { + struct config *conf = ctx->conf; + const char *key = ctx->key; + const char *value = ctx->value; + const char *path = ctx->path; + unsigned lineno = ctx->lineno; + if (strcmp(key, "scaling-filter") == 0) { static const char filters[][12] = { [FCFT_SCALING_FILTER_NONE] = "none", @@ -2544,7 +2617,7 @@ parse_section_tweak( } static bool -parse_key_value(char *kv, char **section, char **key, char **value) +parse_key_value(char *kv, const char **section, const char **key, const char **value) { /*strip leading whitespace*/ @@ -2579,7 +2652,7 @@ parse_key_value(char *kv, char **section, char **key, char **value) { xassert(!isspace(**key)); - char *end = *key + strlen(*key) - 1; + char *end = (char *)*key + strlen(*key) - 1; while (isspace(*end)) end--; *(end + 1) = '\0'; @@ -2591,7 +2664,7 @@ parse_key_value(char *kv, char **section, char **key, char **value) ++*value; if (*value[0] != '\0') { - char *end = *value + strlen(*value) - 1; + char *end = (char *)*value + strlen(*value) - 1; while (isspace(*end)) end--; *(end + 1) = '\0'; @@ -2618,9 +2691,7 @@ enum section { }; /* Function pointer, called for each key/value line */ -typedef bool (*parser_fun_t)( - const char *key, const char *value, struct config *conf, - const char *path, unsigned lineno, bool errors_are_fatal); +typedef bool (*parser_fun_t)(struct context *ctx); static const struct { parser_fun_t fun; @@ -2658,8 +2729,6 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar { enum section section = SECTION_MAIN; - unsigned lineno = 0; - char *_line = NULL; size_t count = 0; @@ -2671,9 +2740,17 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar continue; \ } + struct context ctx = { + .conf = conf, + .section = "main", + .path = path, + .lineno = 0, + .errors_are_fatal = errors_are_fatal, + }; + while (true) { errno = 0; - lineno++; + ctx.lineno++; ssize_t ret = getline(&_line, &count, f); @@ -2720,7 +2797,7 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar if (key_value[0] == '[') { char *end = strchr(key_value, ']'); if (end == NULL) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, lineno, key_value); + LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, ctx.lineno, key_value); error_or_continue(); } @@ -2728,10 +2805,13 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar section = str_to_section(&key_value[1]); if (section == SECTION_COUNT) { - LOG_AND_NOTIFY_ERR("%s:%d: invalid section name: %s", path, lineno, &key_value[1]); + LOG_AND_NOTIFY_ERR("%s:%d: invalid section name: %s", path, ctx.lineno, &key_value[1]); error_or_continue(); } + ctx.section = &key_value[1]; + LOG_INFO("section=\"%s\"", ctx.section); + /* Process next line */ continue; } @@ -2741,9 +2821,8 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar continue; } - char *key, *value; - if (!parse_key_value(key_value, NULL, &key, &value)) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, lineno, key_value); + if (!parse_key_value(key_value, NULL, &ctx.key, &ctx.value)) { + LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, ctx.lineno, key_value); if (errors_are_fatal) goto err; break; @@ -2757,7 +2836,7 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar parser_fun_t section_parser = section_info[section].fun; xassert(section_parser != NULL); - if (!section_parser(key, value, conf, path, lineno, errors_are_fatal)) + if (!section_parser(&ctx)) error_or_continue(); } @@ -3148,20 +3227,26 @@ out: bool config_override_apply(struct config *conf, config_override_t *overrides, bool errors_are_fatal) { - int i = -1; + struct context ctx = { + .conf = conf, + .path = "override", + .lineno = 0, + .errors_are_fatal = errors_are_fatal, + }; + tll_foreach(*overrides, it) { - ++i; - char *section_str, *key, *value; - if (!parse_key_value(it->item, §ion_str, &key, &value)) { + ctx.lineno++; + + if (!parse_key_value(it->item, &ctx.section, &ctx.key, &ctx.value)) { LOG_AND_NOTIFY_ERR("syntax error: %s", it->item); if (errors_are_fatal) return false; continue; } - enum section section = str_to_section(section_str); + enum section section = str_to_section(ctx.section); if (section == SECTION_COUNT) { - LOG_AND_NOTIFY_ERR("override: invalid section name: %s", section_str); + LOG_AND_NOTIFY_ERR("override: invalid section name: %s", ctx.section); if (errors_are_fatal) return false; continue; @@ -3169,7 +3254,7 @@ config_override_apply(struct config *conf, config_override_t *overrides, bool er parser_fun_t section_parser = section_info[section].fun; xassert(section_parser != NULL); - if (!section_parser(key, value, conf, "override", i, errors_are_fatal)) { + if (!section_parser(&ctx)) { if (errors_are_fatal) return false; continue; From 5bb2973c398e26306695d62ef5eb10a6e13bc2cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 13:51:35 +0100 Subject: [PATCH 02/23] config: rename str_to_bool() -> value_to_bool() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change argument to a ‘struct context’ pointer --- config.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/config.c b/config.c index 5f199721..ae5aac6d 100644 --- a/config.c +++ b/config.c @@ -421,8 +421,9 @@ str_has_prefix(const char *str, const char *prefix) } static bool NOINLINE -str_to_bool(const char *s) +value_to_bool(const struct context *ctx) { + const char *s = ctx->value; return strcasecmp(s, "on") == 0 || strcasecmp(s, "true") == 0 || strcasecmp(s, "yes") == 0 || @@ -757,7 +758,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "login-shell") == 0) { - conf->login_shell = str_to_bool(value); + conf->login_shell = value_to_bool(ctx); } else if (strcmp(key, "title") == 0) { @@ -766,7 +767,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "locked-title") == 0) - conf->locked_title = str_to_bool(value); + conf->locked_title = value_to_bool(ctx); else if (strcmp(key, "app-id") == 0) { free(conf->app_id); @@ -839,7 +840,7 @@ parse_section_main(struct context *ctx) conf->bold_in_bright.enabled = true; conf->bold_in_bright.palette_based = true; } else { - conf->bold_in_bright.enabled = str_to_bool(value); + conf->bold_in_bright.enabled = value_to_bool(ctx); conf->bold_in_bright.palette_based = false; } } @@ -954,7 +955,7 @@ parse_section_main(struct context *ctx) if (strcmp(value, "auto") == 0) conf->dpi_aware = DPI_AWARE_AUTO; else - conf->dpi_aware = str_to_bool(value) ? DPI_AWARE_YES : DPI_AWARE_NO; + conf->dpi_aware = value_to_bool(ctx) ? DPI_AWARE_YES : DPI_AWARE_NO; } else if (strcmp(key, "workers") == 0) { @@ -1002,7 +1003,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "notify-focus-inhibit") == 0) { - conf->notify_focus_inhibit = str_to_bool(value); + conf->notify_focus_inhibit = value_to_bool(ctx); } else if (strcmp(key, "url-launch") == 0) { @@ -1055,7 +1056,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "box-drawings-uses-font-glyphs") == 0) - conf->box_drawings_uses_font_glyphs = str_to_bool(value); + conf->box_drawings_uses_font_glyphs = value_to_bool(ctx); else { LOG_AND_NOTIFY_ERR("%s:%u: [main].%s is not a valid option", @@ -1076,15 +1077,15 @@ parse_section_bell(struct context *ctx) unsigned lineno = ctx->lineno; if (strcmp(key, "urgent") == 0) - conf->bell.urgent = str_to_bool(value); + conf->bell.urgent = value_to_bool(ctx); else if (strcmp(key, "notify") == 0) - conf->bell.notify = str_to_bool(value); + conf->bell.notify = value_to_bool(ctx); else if (strcmp(key, "command") == 0) { if (!str_to_spawn_template(conf, value, &conf->bell.command, path, lineno, "bell", key)) return false; } else if (strcmp(key, "command-focused") == 0) - conf->bell.command_focused = str_to_bool(value); + conf->bell.command_focused = value_to_bool(ctx); else { LOG_AND_NOTIFY_ERR("%s:%u: [bell].%s is not a valid option", path, lineno, key); @@ -1421,7 +1422,7 @@ parse_section_cursor(struct context *ctx) } else if (strcmp(key, "blink") == 0) - conf->cursor.blink = str_to_bool(value); + conf->cursor.blink = value_to_bool(ctx); else if (strcmp(key, "color") == 0) { if (!str_to_two_colors( @@ -1463,15 +1464,14 @@ parse_section_mouse(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *value = ctx->value; const char *path = ctx->path; unsigned lineno = ctx->lineno; if (strcmp(key, "hide-when-typing") == 0) - conf->mouse.hide_when_typing = str_to_bool(value); + conf->mouse.hide_when_typing = value_to_bool(ctx); else if (strcmp(key, "alternate-scroll-mode") == 0) - conf->mouse.alternate_scroll_mode = str_to_bool(value); + conf->mouse.alternate_scroll_mode = value_to_bool(ctx); else { LOG_AND_NOTIFY_ERR("%s:%d: [mouse].%s is not a valid option", @@ -2454,19 +2454,19 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "overflowing-glyphs") == 0) { - conf->tweak.overflowing_glyphs = str_to_bool(value); + conf->tweak.overflowing_glyphs = value_to_bool(ctx); if (!conf->tweak.overflowing_glyphs) LOG_WARN("tweak: disabled overflowing glyphs"); } else if (strcmp(key, "damage-whole-window") == 0) { - conf->tweak.damage_whole_window = str_to_bool(value); + conf->tweak.damage_whole_window = value_to_bool(ctx); if (conf->tweak.damage_whole_window) LOG_WARN("tweak: damage whole window"); } else if (strcmp(key, "grapheme-shaping") == 0) { - conf->tweak.grapheme_shaping = str_to_bool(value); + conf->tweak.grapheme_shaping = value_to_bool(ctx); #if !defined(FOOT_GRAPHEME_CLUSTERING) if (conf->tweak.grapheme_shaping) { @@ -2597,7 +2597,7 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "box-drawing-solid-shades") == 0) { - conf->tweak.box_drawing_solid_shades = str_to_bool(value); + conf->tweak.box_drawing_solid_shades = value_to_bool(ctx); if (!conf->tweak.box_drawing_solid_shades) LOG_WARN("tweak: box-drawing-solid-shades=%s", @@ -2605,7 +2605,7 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "font-monospace-warn") == 0) - conf->tweak.font_monospace_warn = str_to_bool(value); + conf->tweak.font_monospace_warn = value_to_bool(ctx); else { LOG_AND_NOTIFY_ERR("%s:%u: [tweak].%s is not a valid option", From 92e08a04ed1d785b3edc3bd43c9e51ea73e50882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:07:54 +0100 Subject: [PATCH 03/23] config: value_to_bool(): fixup --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index ae5aac6d..071bda9b 100644 --- a/config.c +++ b/config.c @@ -421,7 +421,7 @@ str_has_prefix(const char *str, const char *prefix) } static bool NOINLINE -value_to_bool(const struct context *ctx) +value_to_bool(struct context *ctx) { const char *s = ctx->value; return strcasecmp(s, "on") == 0 || From 922490217ea5f08cc0beef8049ac025a87c523f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:08:25 +0100 Subject: [PATCH 04/23] config: rename str_to_ulong() -> value_to_ulong() * str_to_ulong() -> value_to_ulong() * str_to_color() -> value_to_color() * str_to_two_colors() -> value_to_two_colors() --- config.c | 120 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 73 insertions(+), 47 deletions(-) diff --git a/config.c b/config.c index 071bda9b..7bb95ddc 100644 --- a/config.c +++ b/config.c @@ -430,7 +430,7 @@ value_to_bool(struct context *ctx) strtoul(s, NULL, 0) > 0; } -static bool NOINLINE +static bool str_to_ulong(const char *s, int base, unsigned long *res) { if (s == NULL) @@ -443,6 +443,12 @@ str_to_ulong(const char *s, int base, unsigned long *res) return errno == 0 && *end == '\0'; } +static bool NOINLINE +value_to_ulong(struct context *ctx, int base, unsigned long *res) +{ + return str_to_ulong(ctx->value, base, res); +} + static bool NOINLINE str_to_double(const char *s, double *res) { @@ -476,22 +482,23 @@ str_to_wchars(const char *s, wchar_t **res, struct config *conf, } static bool NOINLINE -str_to_color(const char *s, uint32_t *color, bool allow_alpha, - struct config *conf, const char *path, int lineno, - const char *section, const char *key) +value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha) { + /* TODO: remove! */ + struct config *conf = ctx->conf; + unsigned long value; - if (!str_to_ulong(s, 16, &value)) { + if (!value_to_ulong(ctx, 16, &value)) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s is not a valid color value", - path, lineno, section, key, s); + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); return false; } if (!allow_alpha && (value & 0xff000000) != 0) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: color value must not have an alpha component", - path, lineno, section, key, s); + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); return false; } @@ -500,25 +507,41 @@ str_to_color(const char *s, uint32_t *color, bool allow_alpha, } static bool NOINLINE -str_to_two_colors(const char *s, uint32_t *first, uint32_t *second, - bool allow_alpha, struct config *conf, const char *path, - int lineno, const char *section, const char *key) +value_to_two_colors(struct context *ctx, + uint32_t *first, uint32_t *second, bool allow_alpha) { + /* TODO: remove! */ + struct config *conf = ctx->conf; + + bool ret = false; + const char *original_value = ctx->value; + /* TODO: do this without strdup() */ - char *value_copy = xstrdup(s); + char *value_copy = xstrdup(ctx->value); const char *first_as_str = strtok(value_copy, " "); const char *second_as_str = strtok(NULL, " "); - if (first_as_str == NULL || second_as_str == NULL || - !str_to_color(first_as_str, first, allow_alpha, conf, path, lineno, section, key) || - !str_to_color(second_as_str, second, allow_alpha, conf, path, lineno, section, key)) - { - free(value_copy); - return false; + if (first_as_str == NULL || second_as_str == NULL) { + LOG_AND_NOTIFY_ERR("%s:%d: [%s].%s: %s: invalid double color value", + ctx->path, ctx->lineno, ctx->section, ctx->key, + ctx->value); + goto out; } + ctx->value = first_as_str; + if (!value_to_color(ctx, first, allow_alpha)) + goto out; + + ctx->value = second_as_str; + if (!value_to_color(ctx, second, allow_alpha)) + goto out; + + ret = true; + +out: free(value_copy); - return true; + ctx->value = original_value; + return ret; } static bool NOINLINE @@ -825,7 +848,7 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "resize-delay-ms") == 0) { unsigned long ms; - if (!str_to_ulong(value, 10, &ms)) { + if (!value_to_ulong(ctx, 10, &ms)) { LOG_AND_NOTIFY_ERR( "%s:%d: [main].resize-delay-ms: %s: invalid integer value", path, lineno, value); @@ -960,7 +983,7 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "workers") == 0) { unsigned long count; - if (!str_to_ulong(value, 10, &count)) { + if (!value_to_ulong(ctx, 10, &count)) { LOG_AND_NOTIFY_ERR( "%s:%d: [main].workers: %s: invalid integer value", path, lineno, value); @@ -1106,7 +1129,7 @@ parse_section_scrollback(struct context *ctx) if (strcmp(key, "lines") == 0) { unsigned long lines; - if (!str_to_ulong(value, 10, &lines)) { + if (!value_to_ulong(ctx, 10, &lines)) { LOG_AND_NOTIFY_ERR( "%s:%d: [scrollback].lines: %s: invalid integer value", path, lineno, value); @@ -1334,9 +1357,11 @@ parse_section_colors(struct context *ctx) else if (strcmp(key, "selection-background") == 0) color = &conf->colors.selection_bg; else if (strcmp(key, "jump-labels") == 0) { - if (!str_to_two_colors( - value, &conf->colors.jump_label.fg, &conf->colors.jump_label.bg, - false, conf, path, lineno, "colors", "jump-labels")) + if (!value_to_two_colors( + ctx, + &conf->colors.jump_label.fg, + &conf->colors.jump_label.bg, + false)) { return false; } @@ -1346,9 +1371,11 @@ parse_section_colors(struct context *ctx) } else if (strcmp(key, "scrollback-indicator") == 0) { - if (!str_to_two_colors( - value, &conf->colors.scrollback_indicator.fg, &conf->colors.scrollback_indicator.bg, - false, conf, path, lineno, "colors", "scrollback-indicator")) + if (!value_to_two_colors( + ctx, + &conf->colors.scrollback_indicator.fg, + &conf->colors.scrollback_indicator.bg, + false)) { return false; } @@ -1358,11 +1385,8 @@ parse_section_colors(struct context *ctx) } else if (strcmp(key, "urls") == 0) { - if (!str_to_color(value, &conf->colors.url, false, - conf, path, lineno, "colors", "urls")) - { + if (!value_to_color(ctx, &conf->colors.url, false)) return false; - } conf->colors.use_custom.url = true; return true; @@ -1389,7 +1413,7 @@ parse_section_colors(struct context *ctx) } uint32_t color_value; - if (!str_to_color(value, &color_value, false, conf, path, lineno, "colors", key)) + if (!value_to_color(ctx, &color_value, false)) return false; *color = color_value; @@ -1425,9 +1449,11 @@ parse_section_cursor(struct context *ctx) conf->cursor.blink = value_to_bool(ctx); else if (strcmp(key, "color") == 0) { - if (!str_to_two_colors( - value, &conf->cursor.color.text, &conf->cursor.color.cursor, - false, conf, path, lineno, "cursor", "color")) + if (!value_to_two_colors( + ctx, + &conf->cursor.color.text, + &conf->cursor.color.cursor, + false)) { return false; } @@ -1519,7 +1545,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "color") == 0) { uint32_t color; - if (!str_to_color(value, &color, true, conf, path, lineno, "csd", "color")) + if (!value_to_color(ctx, &color, true)) return false; conf->csd.color.title_set = true; @@ -1528,7 +1554,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "size") == 0) { unsigned long pixels; - if (!str_to_ulong(value, 10, &pixels)) { + if (!value_to_ulong(ctx, 10, &pixels)) { LOG_AND_NOTIFY_ERR( "%s:%d: [csd].size: %s: invalid integer value", path, lineno, value); @@ -1540,7 +1566,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "button-width") == 0) { unsigned long pixels; - if (!str_to_ulong(value, 10, &pixels)) { + if (!value_to_ulong(ctx, 10, &pixels)) { LOG_AND_NOTIFY_ERR( "%s:%d: [csd].button-width: %s: invalid integer value", path, lineno, value); @@ -1552,7 +1578,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "button-color") == 0) { uint32_t color; - if (!str_to_color(value, &color, true, conf, path, lineno, "csd", "button-color")) + if (!value_to_color(ctx, &color, true)) return false; conf->csd.color.buttons_set = true; @@ -1561,7 +1587,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "button-minimize-color") == 0) { uint32_t color; - if (!str_to_color(value, &color, true, conf, path, lineno, "csd", "button-minimize-color")) + if (!value_to_color(ctx, &color, true)) return false; conf->csd.color.minimize_set = true; @@ -1570,7 +1596,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "button-maximize-color") == 0) { uint32_t color; - if (!str_to_color(value, &color, true, conf, path, lineno, "csd", "button-maximize-color")) + if (!value_to_color(ctx, &color, true)) return false; conf->csd.color.maximize_set = true; @@ -1579,7 +1605,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "button-close-color") == 0) { uint32_t color; - if (!str_to_color(value, &color, true, conf, path, lineno, "csd", "button-close-color")) + if (!value_to_color(ctx, &color, true)) return false; conf->csd.color.close_set = true; @@ -1588,7 +1614,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "border-color") == 0) { uint32_t color; - if (!str_to_color(value, &color, true, conf, path, lineno, "csd", "border-color")) + if (!value_to_color(ctx, &color, true)) return false; conf->csd.color.border_set = true; @@ -1597,7 +1623,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "border-width") == 0) { unsigned long width; - if (!str_to_ulong(value, 10, &width)) { + if (!value_to_ulong(ctx, 10, &width)) { LOG_AND_NOTIFY_ERR( "%s:%u: [csd].border-width: %s: invalid integer value", path, lineno, value); @@ -2530,7 +2556,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "delayed-render-lower") == 0) { unsigned long ns; - if (!str_to_ulong(value, 10, &ns)) { + if (!value_to_ulong(ctx, 10, &ns)) { LOG_AND_NOTIFY_ERR( "%s:%d: [tweak].delayed-render-lower: %s: invalid integer value", path, lineno, value); @@ -2550,7 +2576,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "delayed-render-upper") == 0) { unsigned long ns; - if (!str_to_ulong(value, 10, &ns)) { + if (!value_to_ulong(ctx, 10, &ns)) { LOG_AND_NOTIFY_ERR( "%s:%d: [tweak].delayed-render-upper: %s: invalid integer value", path, lineno, value); @@ -2570,7 +2596,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "max-shm-pool-size-mb") == 0) { unsigned long mb; - if (!str_to_ulong(value, 10, &mb)) { + if (!value_to_ulong(ctx, 10, &mb)) { LOG_AND_NOTIFY_ERR( "%s:%d: [tweak].max-shm-pool-size-mb: %s: invalid integer value", path, lineno, value); From 5fb86859df9f6201f98ec507585a25acfd11575c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:13:30 +0100 Subject: [PATCH 05/23] config: rename str_to_double() -> value_to_double() * str_to_double() -> value_to_double() * str_to_pt_or_px() -> value_to_pt_or_px() --- config.c | 50 ++++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/config.c b/config.c index 7bb95ddc..0735aac1 100644 --- a/config.c +++ b/config.c @@ -450,8 +450,10 @@ value_to_ulong(struct context *ctx, int base, unsigned long *res) } static bool NOINLINE -str_to_double(const char *s, double *res) +value_to_double(struct context *ctx, double *res) { + const char *s = ctx->value; + if (s == NULL) return false; @@ -545,9 +547,13 @@ out: } static bool NOINLINE -str_to_pt_or_px(const char *s, struct pt_or_px *res, struct config *conf, - const char *path, int lineno, const char *section, const char *key) +value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) { + /* TODO: remove! */ + struct config *conf = ctx->conf; + + const char *s = ctx->value; + size_t len = s != NULL ? strlen(s) : 0; if (len >= 2 && s[len - 2] == 'p' && s[len - 1] == 'x') { errno = 0; @@ -558,17 +564,17 @@ str_to_pt_or_px(const char *s, struct pt_or_px *res, struct config *conf, LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: invalid px value " "(must be on the form 12px)", - path, lineno, section, key, s); + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); return false; } res->pt = 0; res->px = value; } else { double value; - if (!str_to_double(s, &value)) { + if (!value_to_double(ctx, &value)) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: invalid decimal value", - path, lineno, section, key, s); + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); return false; } res->pt = value; @@ -941,35 +947,27 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "line-height") == 0) { - if (!str_to_pt_or_px(value, &conf->line_height, - conf, path, lineno, "main", "line-height")) + if (!value_to_pt_or_px(ctx, &conf->line_height)) return false; } else if (strcmp(key, "letter-spacing") == 0) { - if (!str_to_pt_or_px(value, &conf->letter_spacing, - conf, path, lineno, "main", "letter-spacing")) + if (!value_to_pt_or_px(ctx, &conf->letter_spacing)) return false; } else if (strcmp(key, "horizontal-letter-offset") == 0) { - if (!str_to_pt_or_px( - value, &conf->horizontal_letter_offset, - conf, path, lineno, "main", "horizontal-letter-offset")) + if (!value_to_pt_or_px(ctx, &conf->horizontal_letter_offset)) return false; } else if (strcmp(key, "vertical-letter-offset") == 0) { - if (!str_to_pt_or_px( - value, &conf->vertical_letter_offset, - conf, path, lineno, "main", "vertical-letter-offset")) + if (!value_to_pt_or_px(ctx, &conf->vertical_letter_offset)) return false; } else if (strcmp(key, "underline-offset") == 0) { - if (!str_to_pt_or_px( - value, &conf->underline_offset, - conf, path, lineno, "main", "underline-offset")) + if (!value_to_pt_or_px(ctx, &conf->underline_offset)) return false; conf->use_custom_underline_offset = true; } @@ -1180,7 +1178,7 @@ parse_section_scrollback(struct context *ctx) else if (strcmp(key, "multiplier") == 0) { double multiplier; - if (!str_to_double(value, &multiplier)) { + if (!value_to_double(ctx, &multiplier)) { LOG_AND_NOTIFY_ERR( "%s:%d: [scrollback].multiplier: %s: invalid decimal value", path, lineno, value); @@ -1394,7 +1392,7 @@ parse_section_colors(struct context *ctx) else if (strcmp(key, "alpha") == 0) { double alpha; - if (!str_to_double(value, &alpha) || alpha < 0. || alpha > 1.) { + if (!value_to_double(ctx, &alpha) || alpha < 0. || alpha > 1.) { LOG_AND_NOTIFY_ERR( "%s:%d: [colors].alpha: %s: " "invalid decimal value, or not in range 0.0-1.0", @@ -1463,16 +1461,12 @@ parse_section_cursor(struct context *ctx) } else if (strcmp(key, "beam-thickness") == 0) { - if (!str_to_pt_or_px( - value, &conf->cursor.beam_thickness, - conf, path, lineno, "cursor", "beam-thickness")) + if (!value_to_pt_or_px(ctx, &conf->cursor.beam_thickness)) return false; } else if (strcmp(key, "underline-thickness") == 0) { - if (!str_to_pt_or_px( - value, &conf->cursor.underline_thickness, - conf, path, lineno, "cursor", "underline-thickness")) + if (!value_to_pt_or_px(ctx, &conf->cursor.underline_thickness)) return false; } @@ -2610,7 +2604,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "box-drawing-base-thickness") == 0) { double base_thickness; - if (!str_to_double(value, &base_thickness)) { + if (!value_to_double(ctx, &base_thickness)) { LOG_AND_NOTIFY_ERR( "%s:%d: [tweak].box-drawing-base-thickness: %s: " "invalid decimal value", path, lineno, value); From 4aa3d1d5f838f8b38a15274fe955533350bbc41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:15:55 +0100 Subject: [PATCH 06/23] config: rename str_to_wchars() -> value_to_wchars() --- config.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/config.c b/config.c index 0735aac1..f4b1e55f 100644 --- a/config.c +++ b/config.c @@ -465,21 +465,22 @@ value_to_double(struct context *ctx, double *res) } static bool NOINLINE -str_to_wchars(const char *s, wchar_t **res, struct config *conf, - const char *path, int lineno, - const char *section, const char *key) +value_to_wchars(struct context *ctx, wchar_t **res) { + /* TODO: remove! */ + struct config *conf = ctx->conf; + *res = NULL; - size_t chars = mbstowcs(NULL, s, 0); + size_t chars = mbstowcs(NULL, ctx->value, 0); if (chars == (size_t)-1) { LOG_AND_NOTIFY_ERR("%s:%d: [%s].%s: %s is not a valid string value", - path, lineno, section, key, s); + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); return false; } *res = xmalloc((chars + 1) * sizeof(wchar_t)); - mbstowcs(*res, s, chars + 1); + mbstowcs(*res, ctx->value, chars + 1); return true; } @@ -992,11 +993,9 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "word-delimiters") == 0) { wchar_t *word_delimiters; - if (!str_to_wchars(value, &word_delimiters, conf, path, lineno, - "main", "word-delimiters")) - { + if (!value_to_wchars(ctx, &word_delimiters)) return false; - } + free(conf->word_delimiters); conf->word_delimiters = word_delimiters; } @@ -1006,11 +1005,9 @@ parse_section_main(struct context *ctx) conf, "jump-label-letters", "label-letters", path, lineno); wchar_t *letters; - if (!str_to_wchars(value, &letters, conf, path, lineno, - "main", "label-letters")) - { + if (!value_to_wchars(ctx, &letters)) return false; - } + free(conf->url.label_letters); conf->url.label_letters = letters; } @@ -1216,7 +1213,7 @@ parse_section_url(struct context *ctx) else if (strcmp(key, "label-letters") == 0) { wchar_t *letters; - if (!str_to_wchars(value, &letters, conf, path, lineno, "url", "letters")) + if (!value_to_wchars(ctx, &letters)) return false; free(conf->url.label_letters); @@ -1289,11 +1286,8 @@ parse_section_url(struct context *ctx) else if (strcmp(key, "uri-characters") == 0) { wchar_t *uri_characters; - if (!str_to_wchars(value, &uri_characters, conf, path, lineno, - "url", "uri-characters")) - { + if (!value_to_wchars(ctx, &uri_characters)) return false; - } free(conf->url.uri_characters); From 70aec2068a89c46263e81ced7b06c5d9e169e1ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:17:42 +0100 Subject: [PATCH 07/23] config: rename str_to_fonts() -> value_to_fonts() --- config.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/config.c b/config.c index f4b1e55f..08a751fc 100644 --- a/config.c +++ b/config.c @@ -586,14 +586,16 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) } static struct config_font_list NOINLINE -str_to_fonts(const char *s, struct config *conf, const char *path, int lineno, - const char *section, const char *key) +value_to_fonts(struct context *ctx) { + /* TODO: remove! */ + struct config *conf = ctx->conf; + size_t count = 0; size_t size = 0; struct config_font *fonts = NULL; - char *copy = xstrdup(s); + char *copy = xstrdup(ctx->value); for (const char *font = strtok(copy, ","); font != NULL; font = strtok(NULL, ",")) @@ -609,7 +611,7 @@ str_to_fonts(const char *s, struct config *conf, const char *path, int lineno, if (!config_font_parse(font, &font_data)) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: invalid font specification", - path, lineno, section, key, font); + ctx->path, ctx->lineno, ctx->section, ctx->key, font); goto err; } @@ -937,9 +939,7 @@ parse_section_main(struct context *ctx) strcmp(key, "font-bold") == 0 ? 1 : strcmp(key, "font-italic") == 0 ? 2 : 3; - struct config_font_list new_list = str_to_fonts( - value, conf, path, lineno, "main", key); - + struct config_font_list new_list = value_to_fonts(ctx); if (new_list.arr == NULL) return false; @@ -1521,9 +1521,7 @@ parse_section_csd(struct context *ctx) } else if (strcmp(key, "font") == 0) { - struct config_font_list new_list = str_to_fonts( - value, conf, path, lineno, "csd", "font"); - + struct config_font_list new_list = value_to_fonts(ctx); if (new_list.arr == NULL) return false; From 958ef9dd3bb0ddf3af25a786bd5076a53640c75b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:19:48 +0100 Subject: [PATCH 08/23] config: rename str_to_spawn_template() -> value_to_spawn_template() --- config.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/config.c b/config.c index 08a751fc..cdb5b73e 100644 --- a/config.c +++ b/config.c @@ -680,19 +680,20 @@ spawn_template_clone(struct config_spawn_template *dst, } static bool NOINLINE -str_to_spawn_template(struct config *conf, - const char *s, struct config_spawn_template *template, - const char *path, int lineno, const char *section, - const char *key) +value_to_spawn_template(struct context *ctx, + struct config_spawn_template *template) { + /* TODO: remove */ + struct config *conf = ctx->conf; + spawn_template_free(template); char **argv = NULL; - if (!tokenize_cmdline(s, &argv)) { + if (!tokenize_cmdline(ctx->value, &argv)) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: syntax error in command line", - path, lineno, section, key, s); + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); return false; } @@ -1013,11 +1014,8 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "notify") == 0) { - if (!str_to_spawn_template(conf, value, &conf->notify, path, lineno, - "main", "notify")) - { + if (!value_to_spawn_template(ctx, &conf->notify)) return false; - } } else if (strcmp(key, "notify-focus-inhibit") == 0) { @@ -1028,11 +1026,8 @@ parse_section_main(struct context *ctx) deprecated_url_option( conf, "url-launch", "launch", path, lineno); - if (!str_to_spawn_template(conf, value, &conf->url.launch, path, lineno, - "main", "url-launch")) - { + if (!value_to_spawn_template(ctx, &conf->url.launch)) return false; - } } else if (strcmp(key, "selection-target") == 0) { @@ -1090,7 +1085,6 @@ parse_section_bell(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *value = ctx->value; const char *path = ctx->path; unsigned lineno = ctx->lineno; @@ -1099,7 +1093,7 @@ parse_section_bell(struct context *ctx) else if (strcmp(key, "notify") == 0) conf->bell.notify = value_to_bool(ctx); else if (strcmp(key, "command") == 0) { - if (!str_to_spawn_template(conf, value, &conf->bell.command, path, lineno, "bell", key)) + if (!value_to_spawn_template(ctx, &conf->bell.command)) return false; } else if (strcmp(key, "command-focused") == 0) @@ -1204,11 +1198,8 @@ parse_section_url(struct context *ctx) unsigned lineno = ctx->lineno; if (strcmp(key, "launch") == 0) { - if (!str_to_spawn_template(conf, value, &conf->url.launch, path, lineno, - "url", "launch")) - { + if (!value_to_spawn_template(ctx, &conf->url.launch)) return false; - } } else if (strcmp(key, "label-letters") == 0) { From 534e9d8befd8d6a38909ad7354c8ec7c63cf6fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 14:48:06 +0100 Subject: [PATCH 09/23] =?UTF-8?q?config:=20first=20argument=20to=20log=5Fa?= =?UTF-8?q?nd=5Fnotify()=20is=20now=20a=20=E2=80=98struct=20context?= =?UTF-8?q?=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- config.c | 258 +++++++++++++++++++++++++------------------------------ 1 file changed, 119 insertions(+), 139 deletions(-) diff --git a/config.c b/config.c index cdb5b73e..c4fbd249 100644 --- a/config.c +++ b/config.c @@ -134,7 +134,7 @@ struct context { }; static void NOINLINE PRINTF(5) -log_and_notify(struct config *conf, enum log_class log_class, +log_and_notify(struct context *ctx, enum log_class log_class, const char *file, int lineno, const char *fmt, ...) { enum user_notification_kind kind; @@ -159,7 +159,7 @@ log_and_notify(struct config *conf, enum log_class log_class, char *text = xvasprintf(fmt, va2); tll_push_back( - conf->notifications, + ctx->conf->notifications, ((struct user_notification){.kind = kind, .text = text})); va_end(va2); @@ -167,7 +167,7 @@ log_and_notify(struct config *conf, enum log_class log_class, } static void NOINLINE PRINTF(5) -log_errno_and_notify(struct config *conf, enum log_class log_class, +log_errno_and_notify(struct context *ctx, enum log_class log_class, const char *file, int lineno, const char *fmt, ...) { int errno_copy = errno; @@ -188,7 +188,7 @@ log_errno_and_notify(struct config *conf, enum log_class log_class, snprintf(&text[len], errno_len + 1, ": %s", strerror(errno_copy)); tll_push_back( - conf->notifications, + ctx->conf->notifications, ((struct user_notification){ .kind = USER_NOTIFICATION_ERROR, .text = text})); @@ -198,13 +198,13 @@ log_errno_and_notify(struct config *conf, enum log_class log_class, } #define LOG_AND_NOTIFY_ERR(...) \ - log_and_notify(conf, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) + log_and_notify(ctx, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) #define LOG_AND_NOTIFY_WARN(...) \ - log_and_notify(conf, LOG_CLASS_WARNING, __FILE__, __LINE__, __VA_ARGS__) + log_and_notify(ctx, LOG_CLASS_WARNING, __FILE__, __LINE__, __VA_ARGS__) #define LOG_AND_NOTIFY_ERRNO(...) \ - log_errno_and_notify(conf, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) + log_errno_and_notify(ctx, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) static char * get_shell(void) @@ -467,9 +467,6 @@ value_to_double(struct context *ctx, double *res) static bool NOINLINE value_to_wchars(struct context *ctx, wchar_t **res) { - /* TODO: remove! */ - struct config *conf = ctx->conf; - *res = NULL; size_t chars = mbstowcs(NULL, ctx->value, 0); @@ -487,9 +484,6 @@ value_to_wchars(struct context *ctx, wchar_t **res) static bool NOINLINE value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha) { - /* TODO: remove! */ - struct config *conf = ctx->conf; - unsigned long value; if (!value_to_ulong(ctx, 16, &value)) { LOG_AND_NOTIFY_ERR( @@ -513,9 +507,6 @@ static bool NOINLINE value_to_two_colors(struct context *ctx, uint32_t *first, uint32_t *second, bool allow_alpha) { - /* TODO: remove! */ - struct config *conf = ctx->conf; - bool ret = false; const char *original_value = ctx->value; @@ -550,9 +541,6 @@ out: static bool NOINLINE value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) { - /* TODO: remove! */ - struct config *conf = ctx->conf; - const char *s = ctx->value; size_t len = s != NULL ? strlen(s) : 0; @@ -588,9 +576,6 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) static struct config_font_list NOINLINE value_to_fonts(struct context *ctx) { - /* TODO: remove! */ - struct config *conf = ctx->conf; - size_t count = 0; size_t size = 0; struct config_font *fonts = NULL; @@ -683,9 +668,6 @@ static bool NOINLINE value_to_spawn_template(struct context *ctx, struct config_spawn_template *template) { - /* TODO: remove */ - struct config *conf = ctx->conf; - spawn_template_free(template); char **argv = NULL; @@ -1648,8 +1630,8 @@ free_key_combo_list(struct key_combo_list *key_combos) } static bool -parse_modifiers(struct config *conf, const char *text, size_t len, - struct config_key_modifiers *modifiers, const char *path, unsigned lineno) +parse_modifiers(struct context *ctx, const char *text, size_t len, + struct config_key_modifiers *modifiers) { bool ret = false; @@ -1670,7 +1652,7 @@ parse_modifiers(struct config *conf, const char *text, size_t len, modifiers->meta = true; else { LOG_AND_NOTIFY_ERR("%s:%d: %s: not a valid modifier name", - path, lineno, key); + ctx->path, ctx->lineno, ctx->key); goto out; } } @@ -1683,17 +1665,14 @@ out: } static bool -parse_key_combos(struct config *conf, const char *combos, - struct key_combo_list *key_combos, - const char *section, const char *option, - const char *path, unsigned lineno) +value_to_key_combos(struct context *ctx, struct key_combo_list *key_combos) { xassert(key_combos != NULL); xassert(key_combos->count == 0 && key_combos->combos == NULL); size_t size = 0; /* Size of ‘combos’ array in the key-combo list */ - char *copy = xstrdup(combos); + char *copy = xstrdup(ctx->value); for (char *tok_ctx = NULL, *combo = strtok_r(copy, " ", &tok_ctx); combo != NULL; @@ -1706,7 +1685,7 @@ parse_key_combos(struct config *conf, const char *combos, /* No modifiers */ key = combo; } else { - if (!parse_modifiers(conf, combo, key - combo, &modifiers, path, lineno)) + if (!parse_modifiers(ctx, combo, key - combo, &modifiers)) goto err; key++; /* Skip past the '+' */ } @@ -1731,7 +1710,7 @@ parse_key_combos(struct config *conf, const char *combos, if (sym == XKB_KEY_NoSymbol) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: is not a valid XKB key name", - path, lineno, section, option, key); + ctx->path, ctx->lineno, ctx->section, ctx->key, key); goto err; } @@ -1759,11 +1738,10 @@ err: } static bool -has_key_binding_collisions(struct config *conf, +has_key_binding_collisions(struct context *ctx, int action, const char *const action_map[], const struct config_key_binding_list *bindings, - const struct key_combo_list *key_combos, - const char *path, unsigned lineno) + const struct key_combo_list *key_combos) { for (size_t j = 0; j < bindings->count; j++) { const struct config_key_binding *combo1 = &bindings->arr[j]; @@ -1789,7 +1767,7 @@ has_key_binding_collisions(struct config *conf, if (shift && alt && ctrl && meta && sym) { bool has_pipe = combo1->pipe.argv.args != NULL; LOG_AND_NOTIFY_ERR("%s:%d: %s already mapped to '%s%s%s%s'", - path, lineno, combo2->text, + ctx->path, ctx->lineno, combo2->text, action_map[combo1->action], has_pipe ? " [" : "", has_pipe ? combo1->pipe.argv.args[0] : "", @@ -1847,34 +1825,32 @@ argv_compare(char *const *argv1, char *const *argv2) * - argv: allocatd array containing {"cmd", "arg1", "arg2", NULL}. Caller frees. */ static ssize_t -pipe_argv_from_string(const char *value, char ***argv, - struct config *conf, - const char *path, unsigned lineno) +pipe_argv_from_value(struct context *ctx, char ***argv) { *argv = NULL; - if (value[0] != '[') + if (ctx->value[0] != '[') return 0; - const char *pipe_cmd_end = strrchr(value, ']'); + const char *pipe_cmd_end = strrchr(ctx->value, ']'); if (pipe_cmd_end == NULL) { - LOG_AND_NOTIFY_ERR("%s:%d: unclosed '['", path, lineno); + LOG_AND_NOTIFY_ERR("%s:%d: unclosed '['", ctx->path, ctx->lineno); return -1; } - size_t pipe_len = pipe_cmd_end - value - 1; - char *cmd = xstrndup(&value[1], pipe_len); + size_t pipe_len = pipe_cmd_end - ctx->value - 1; + char *cmd = xstrndup(&ctx->value[1], pipe_len); if (!tokenize_cmdline(cmd, argv)) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error in command line", path, lineno); + LOG_AND_NOTIFY_ERR("%s:%d: syntax error in command line", ctx->path, ctx->lineno); free(cmd); return -1; } - ssize_t remove_len = pipe_cmd_end + 1 - value; - value = pipe_cmd_end + 1; - while (isspace(*value)) { - value++; + ssize_t remove_len = pipe_cmd_end + 1 - ctx->value; + ctx->value = pipe_cmd_end + 1; + while (isspace(*ctx->value)) { + ctx->value++; remove_len++; } @@ -1920,42 +1896,37 @@ remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, } static bool NOINLINE -parse_key_binding_section( - const char *section, const char *key, const char *value, - int action_count, const char *const action_map[static action_count], - struct config_key_binding_list *bindings, - struct config *conf, const char *path, unsigned lineno) +parse_key_binding_section(struct context *ctx, + int action_count, + const char *const action_map[static action_count], + struct config_key_binding_list *bindings) { char **pipe_argv; - ssize_t pipe_remove_len = pipe_argv_from_string( - value, &pipe_argv, conf, path, lineno); - + ssize_t pipe_remove_len = pipe_argv_from_value(ctx, &pipe_argv); if (pipe_remove_len < 0) return false; - value += pipe_remove_len; + ctx->value += pipe_remove_len; for (int action = 0; action < action_count; action++) { if (action_map[action] == NULL) continue; - if (strcmp(key, action_map[action]) != 0) + if (strcmp(ctx->key, action_map[action]) != 0) continue; /* Unset binding */ - if (strcasecmp(value, "none") == 0) { + if (strcasecmp(ctx->value, "none") == 0) { remove_action_from_key_bindings_list(bindings, action, pipe_argv); free(pipe_argv); return true; } struct key_combo_list key_combos = {0}; - if (!parse_key_combos( - conf, value, &key_combos, section, key, path, lineno) || + if (!value_to_key_combos(ctx, &key_combos) || has_key_binding_collisions( - conf, action, action_map, bindings, &key_combos, - path, lineno)) + ctx, action, action_map, bindings, &key_combos)) { free(pipe_argv); free_key_combo_list(&key_combos); @@ -1995,7 +1966,7 @@ parse_key_binding_section( } LOG_AND_NOTIFY_ERR("%s:%u: [%s].%s is not a valid action", - path, lineno, section, key); + ctx->path, ctx->lineno, ctx->section, ctx->key); free(pipe_argv); return false; } @@ -2018,13 +1989,20 @@ UNITTEST struct config conf = {0}; struct config_key_binding_list bindings = {0}; + struct context ctx = { + .conf = &conf, + .section = "", + .key = "foo", + .value = "Escape", + .path = "", + }; + /* * ADD foo=Escape * * This verifies we can bind a single key combo to an action. */ - xassert(parse_key_binding_section( - "", "foo", "Escape", ALEN(map), map, &bindings, &conf, "", 0)); + xassert(parse_key_binding_section(&ctx, ALEN(map), map, &bindings)); xassert(bindings.count == 1); xassert(bindings.arr[0].action == TEST_ACTION_FOO); xassert(bindings.arr[0].sym == XKB_KEY_Escape); @@ -2034,9 +2012,9 @@ UNITTEST * * This verifies we can bind multiple key combos to an action. */ - xassert(parse_key_binding_section( - "", "bar", "Control+g Control+Shift+x", ALEN(map), map, - &bindings, &conf, "", 0)); + ctx.key = "bar"; + ctx.value = "Control+g Control+Shift+x"; + xassert(parse_key_binding_section(&ctx, ALEN(map), map, &bindings)); xassert(bindings.count == 3); xassert(bindings.arr[0].action == TEST_ACTION_FOO); xassert(bindings.arr[1].action == TEST_ACTION_BAR); @@ -2052,9 +2030,9 @@ UNITTEST * This verifies we can update a single-combo action with multiple * key combos. */ - xassert(parse_key_binding_section( - "", "foo", "Mod1+v Shift+q", ALEN(map), map, - &bindings, &conf, "", 0)); + ctx.key = "foo"; + ctx.value = "Mod1+v Shift+q"; + xassert(parse_key_binding_section(&ctx, ALEN(map), map, &bindings)); xassert(bindings.count == 4); xassert(bindings.arr[0].action == TEST_ACTION_BAR); xassert(bindings.arr[1].action == TEST_ACTION_BAR); @@ -2068,8 +2046,9 @@ UNITTEST /* * REMOVE bar */ - xassert(parse_key_binding_section( - "", "bar", "none", ALEN(map), map, &bindings, &conf, "", 0)); + ctx.key = "bar"; + ctx.value = "none"; + xassert(parse_key_binding_section(&ctx, ALEN(map), map, &bindings)); xassert(bindings.count == 2); xassert(bindings.arr[0].action == TEST_ACTION_FOO); xassert(bindings.arr[1].action == TEST_ACTION_FOO); @@ -2077,8 +2056,9 @@ UNITTEST /* * REMOVE foo */ - xassert(parse_key_binding_section( - "", "foo", "none", ALEN(map), map, &bindings, &conf, "", 0)); + ctx.key = "foo"; + ctx.value = "none"; + xassert(parse_key_binding_section(&ctx, ALEN(map), map, &bindings)); xassert(bindings.count == 0); free(bindings.arr); @@ -2087,26 +2067,15 @@ UNITTEST static bool parse_section_key_bindings(struct context *ctx) { - struct config *conf = ctx->conf; - const char *key = ctx->key; - const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; - return parse_key_binding_section( - "key-bindings", key, value, BIND_ACTION_KEY_COUNT, binding_action_map, - &conf->bindings.key, conf, path, lineno); + ctx, + BIND_ACTION_KEY_COUNT, binding_action_map, + &ctx->conf->bindings.key); } static bool parse_section_search_bindings(struct context *ctx) { - struct config *conf = ctx->conf; - const char *key = ctx->key; - const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; - static const char *const search_binding_action_map[] = { [BIND_ACTION_SEARCH_NONE] = NULL, [BIND_ACTION_SEARCH_CANCEL] = "cancel", @@ -2133,19 +2102,14 @@ parse_section_search_bindings(struct context *ctx) "search binding action map size mismatch"); return parse_key_binding_section( - "search-bindings", key, value, BIND_ACTION_SEARCH_COUNT, - search_binding_action_map, &conf->bindings.search, conf, path, lineno); + ctx, + BIND_ACTION_SEARCH_COUNT, search_binding_action_map, + &ctx->conf->bindings.search); } static bool parse_section_url_bindings(struct context *ctx) { - struct config *conf = ctx->conf; - const char *key = ctx->key; - const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; - static const char *const url_binding_action_map[] = { [BIND_ACTION_URL_NONE] = NULL, [BIND_ACTION_URL_CANCEL] = "cancel", @@ -2156,22 +2120,20 @@ parse_section_url_bindings(struct context *ctx) "URL binding action map size mismatch"); return parse_key_binding_section( - "url-bindings", key, value, BIND_ACTION_URL_COUNT, - url_binding_action_map, &conf->bindings.url, conf, path, lineno); + ctx, + BIND_ACTION_URL_COUNT, url_binding_action_map, + &ctx->conf->bindings.url); } static bool -parse_mouse_combos(struct config *conf, const char *combos, - struct key_combo_list *key_combos, - const char *path, unsigned lineno, - const char *section, const char *conf_key) +value_to_mouse_combos(struct context *ctx, struct key_combo_list *key_combos) { xassert(key_combos != NULL); xassert(key_combos->count == 0 && key_combos->combos == NULL); size_t size = 0; /* Size of the ‘combos’ array in key_combos */ - char *copy = xstrdup(combos); + char *copy = xstrdup(ctx->value); for (char *tok_ctx = NULL, *combo = strtok_r(copy, " ", &tok_ctx); combo != NULL; @@ -2185,12 +2147,12 @@ parse_mouse_combos(struct config *conf, const char *combos, key = combo; } else { *key = '\0'; - if (!parse_modifiers(conf, combo, key - combo, &modifiers, path, lineno)) + if (!parse_modifiers(ctx, combo, key - combo, &modifiers)) goto err; if (modifiers.shift) { LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: Shift cannot be used in mouse bindings", - path, lineno, section, conf_key); + ctx->path, ctx->lineno, ctx->section, ctx->key); goto err; } key++; /* Skip past the '+' */ @@ -2209,12 +2171,12 @@ parse_mouse_combos(struct config *conf, const char *combos, if (_count[0] == '\0' || *end != '\0' || errno != 0) { if (errno != 0) LOG_AND_NOTIFY_ERRNO( - "%s:%d: [%s].%s: %s: invalid click count" - , path, lineno, section, conf_key, _count); + "%s:%d: [%s].%s: %s: invalid click count", + ctx->path, ctx->lineno, ctx->section, ctx->key, _count); else LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s: invalid click count", - path, lineno, section, conf_key, _count); + ctx->path, ctx->lineno, ctx->section, ctx->key, _count); goto err; } count = value; @@ -2245,7 +2207,7 @@ parse_mouse_combos(struct config *conf, const char *combos, if (button == 0) { LOG_AND_NOTIFY_ERR("%s:%d: [%s].%s: %s: invalid mouse button name", - path, lineno, section, conf_key, key); + ctx->path, ctx->lineno, ctx->section, ctx->key, key); goto err; } @@ -2278,9 +2240,11 @@ err: } static bool -has_mouse_binding_collisions(struct config *conf, const struct key_combo_list *key_combos, - const char *path, unsigned lineno) +has_mouse_binding_collisions(struct context *ctx, + const struct key_combo_list *key_combos) { + struct config *conf = ctx->conf; + for (size_t j = 0; j < conf->bindings.mouse.count; j++) { const struct config_mouse_binding *combo1 = &conf->bindings.mouse.arr[j]; if (combo1->action == BIND_ACTION_NONE) @@ -2302,7 +2266,7 @@ has_mouse_binding_collisions(struct config *conf, const struct key_combo_list *k if (shift && alt && ctrl && meta && button && count) { bool has_pipe = combo1->pipe.argv.args != NULL; LOG_AND_NOTIFY_ERR("%s:%d: %s already mapped to '%s%s%s%s'", - path, lineno, combo2->text, + ctx->path, ctx->lineno, combo2->text, binding_action_map[combo1->action], has_pipe ? " [" : "", has_pipe ? combo1->pipe.argv.args[0] : "", @@ -2327,9 +2291,7 @@ parse_section_mouse_bindings(struct context *ctx) char **pipe_argv; - ssize_t pipe_remove_len = pipe_argv_from_string( - value, &pipe_argv, conf, path, lineno); - + ssize_t pipe_remove_len = pipe_argv_from_value(ctx, &pipe_argv); if (pipe_remove_len < 0) return false; @@ -2362,9 +2324,8 @@ parse_section_mouse_bindings(struct context *ctx) } struct key_combo_list key_combos = {0}; - if (!parse_mouse_combos( - conf, value, &key_combos, path, lineno, "mouse-bindings", key) || - has_mouse_binding_collisions(conf, &key_combos, path, lineno)) + if (!value_to_mouse_combos(ctx, &key_combos) || + has_mouse_binding_collisions(ctx, &key_combos)) { free(pipe_argv); free_key_combo_list(&key_combos); @@ -2743,17 +2704,18 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar continue; \ } - struct context ctx = { + struct context context = { .conf = conf, .section = "main", .path = path, .lineno = 0, .errors_are_fatal = errors_are_fatal, }; + struct context *ctx = &context; /* For LOG_AND_*() */ while (true) { errno = 0; - ctx.lineno++; + context.lineno++; ssize_t ret = getline(&_line, &count, f); @@ -2800,7 +2762,7 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar if (key_value[0] == '[') { char *end = strchr(key_value, ']'); if (end == NULL) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, ctx.lineno, key_value); + LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, context.lineno, key_value); error_or_continue(); } @@ -2808,12 +2770,11 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar section = str_to_section(&key_value[1]); if (section == SECTION_COUNT) { - LOG_AND_NOTIFY_ERR("%s:%d: invalid section name: %s", path, ctx.lineno, &key_value[1]); + LOG_AND_NOTIFY_ERR("%s:%d: invalid section name: %s", path, context.lineno, &key_value[1]); error_or_continue(); } - ctx.section = &key_value[1]; - LOG_INFO("section=\"%s\"", ctx.section); + context.section = &key_value[1]; /* Process next line */ continue; @@ -2824,8 +2785,8 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar continue; } - if (!parse_key_value(key_value, NULL, &ctx.key, &ctx.value)) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, ctx.lineno, key_value); + if (!parse_key_value(key_value, NULL, &context.key, &context.value)) { + LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, context.lineno, key_value); if (errors_are_fatal) goto err; break; @@ -2839,7 +2800,7 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar parser_fun_t section_parser = section_info[section].fun; xassert(section_parser != NULL); - if (!section_parser(&ctx)) + if (!section_parser(ctx)) error_or_continue(); } @@ -3156,6 +3117,21 @@ config_load(struct config *conf, const char *conf_path, add_default_url_bindings(conf); add_default_mouse_bindings(conf); + /* + * TODO: replace LOG_AND_NOTIFY_*() with custom loggers that + * doesn’t take a context + */ + struct context context = { + .conf = conf, + .section = "", + .key = "", + .value = "", + .path = conf_path, + .lineno = 0, + .errors_are_fatal = errors_are_fatal, + }; + struct context *ctx = &context; + struct config_file conf_file = {.path = NULL, .fd = -1}; if (conf_path != NULL) { int fd = open(conf_path, O_RDONLY); @@ -3230,26 +3206,30 @@ out: bool config_override_apply(struct config *conf, config_override_t *overrides, bool errors_are_fatal) { - struct context ctx = { + struct context context = { .conf = conf, .path = "override", .lineno = 0, .errors_are_fatal = errors_are_fatal, }; + struct context *ctx = &context; tll_foreach(*overrides, it) { - ctx.lineno++; + context.lineno++; - if (!parse_key_value(it->item, &ctx.section, &ctx.key, &ctx.value)) { + if (!parse_key_value( + it->item, &context.section, &context.key, &context.value)) + { LOG_AND_NOTIFY_ERR("syntax error: %s", it->item); if (errors_are_fatal) return false; continue; } - enum section section = str_to_section(ctx.section); + enum section section = str_to_section(context.section); if (section == SECTION_COUNT) { - LOG_AND_NOTIFY_ERR("override: invalid section name: %s", ctx.section); + LOG_AND_NOTIFY_ERR( + "override: invalid section name: %s", context.section); if (errors_are_fatal) return false; continue; @@ -3257,7 +3237,7 @@ config_override_apply(struct config *conf, config_override_t *overrides, bool er parser_fun_t section_parser = section_info[section].fun; xassert(section_parser != NULL); - if (!section_parser(&ctx)) { + if (!section_parser(ctx)) { if (errors_are_fatal) return false; continue; From 176b85cb10a6cafe9376e5bfa99206a3a9feacad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 17:13:39 +0100 Subject: [PATCH 10/23] config: add LOG_CONTEXTUAL_{ERR,WARN,ERRNO} These work like LOG_AND_NOTIFY_* but add contextual information: path of the conf file, line number, section name, key/option name and the value. --- config.c | 462 ++++++++++++++++++++----------------------------------- log.h | 3 +- 2 files changed, 170 insertions(+), 295 deletions(-) diff --git a/config.c b/config.c index c4fbd249..8ea0d8c0 100644 --- a/config.c +++ b/config.c @@ -133,78 +133,113 @@ struct context { bool errors_are_fatal; }; -static void NOINLINE PRINTF(5) -log_and_notify(struct context *ctx, enum log_class log_class, - const char *file, int lineno, const char *fmt, ...) +static const enum user_notification_kind log_class_to_notify_kind[LOG_CLASS_COUNT] = { + [LOG_CLASS_WARNING] = USER_NOTIFICATION_WARNING, + [LOG_CLASS_ERROR] = USER_NOTIFICATION_ERROR, +}; + +static void NOINLINE VPRINTF(5) +log_and_notify_va(struct config *conf, enum log_class log_class, + const char *file, int lineno, const char *fmt, va_list va) { - enum user_notification_kind kind; + xassert(log_class < ALEN(log_class_to_notify_kind)); + enum user_notification_kind kind = log_class_to_notify_kind[log_class]; - switch (log_class) { - case LOG_CLASS_WARNING: kind = USER_NOTIFICATION_WARNING; break; - case LOG_CLASS_ERROR: kind = USER_NOTIFICATION_ERROR; break; - - case LOG_CLASS_INFO: - case LOG_CLASS_DEBUG: - case LOG_CLASS_NONE: - default: + if (kind == 0) { BUG("unsupported log class: %d", (int)log_class); return; } - va_list va1, va2; - va_start(va1, fmt); - va_copy(va2, va1); - - log_msg_va(log_class, LOG_MODULE, file, lineno, fmt, va1); - - char *text = xvasprintf(fmt, va2); + char *formatted_msg = xvasprintf(fmt, va); + log_msg(log_class, LOG_MODULE, file, lineno, formatted_msg); tll_push_back( - ctx->conf->notifications, - ((struct user_notification){.kind = kind, .text = text})); - - va_end(va2); - va_end(va1); + conf->notifications, + ((struct user_notification){.kind = kind, .text = formatted_msg})); } static void NOINLINE PRINTF(5) -log_errno_and_notify(struct context *ctx, enum log_class log_class, - const char *file, int lineno, const char *fmt, ...) +log_and_notify(struct config *conf, enum log_class log_class, + const char *file, int lineno, const char *fmt, ...) { - int errno_copy = errno; - - va_list va1, va2, va3; - va_start(va1, fmt); - va_copy(va2, va1); - va_copy(va3, va2); - - log_errno_provided_va( - log_class, LOG_MODULE, file, lineno, errno_copy, fmt, va1); - - int len = vsnprintf(NULL, 0, fmt, va2); - int errno_len = snprintf(NULL, 0, ": %s", strerror(errno_copy)); - - char *text = xmalloc(len + errno_len + 1); - vsnprintf(text, len + errno_len + 1, fmt, va3); - snprintf(&text[len], errno_len + 1, ": %s", strerror(errno_copy)); - - tll_push_back( - ctx->conf->notifications, - ((struct user_notification){ - .kind = USER_NOTIFICATION_ERROR, .text = text})); - - va_end(va3); - va_end(va2); - va_end(va1); + va_list va; + va_start(va, fmt); + log_and_notify_va(conf, log_class, file, lineno, fmt, va); + va_end(va); } +static void NOINLINE PRINTF(5) +log_contextual(struct context *ctx, enum log_class log_class, + const char *file, int lineno, const char *fmt, ...) +{ + va_list va; + va_start(va, fmt); + char *formatted_msg = xvasprintf(fmt, va); + va_end(va); + + 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); + free(formatted_msg); +} + + +static void NOINLINE VPRINTF(4) +log_and_notify_errno_va(struct config *conf, const char *file, int lineno, + const char *fmt, va_list va) +{ + int errno_copy = errno; + char *formatted_msg = xvasprintf(fmt, va); + log_and_notify( + conf, LOG_CLASS_ERROR, file, lineno, + "%s: %s", formatted_msg, strerror(errno_copy)); + free(formatted_msg); +} + +static void NOINLINE PRINTF(4) +log_and_notify_errno(struct config *conf, const char *file, int lineno, + const char *fmt, ...) +{ + va_list va; + va_start(va, fmt); + log_and_notify_errno_va(conf, file, lineno, fmt, va); + va_end(va); +} + +static void NOINLINE PRINTF(4) +log_contextual_errno(struct context *ctx, const char *file, int lineno, + const char *fmt, ...) +{ + va_list va; + va_start(va, fmt); + char *formatted_msg = xvasprintf(fmt, va); + va_end(va); + + log_and_notify_errno( + ctx->conf, file, lineno, "%s:%d: [%s].%s: %s: %s", + ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value, + formatted_msg); + + free(formatted_msg); +} + +#define LOG_CONTEXTUAL_ERR(...) \ + log_contextual(ctx, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) + +#define LOG_CONTEXTUAL_WARN(...) \ + log_contextual(ctx, LOG_CLASS_WARNING, __FILE__, __LINE__, __VA_ARGS__) + +#define LOG_CONTEXTUAL_ERRNO(...) \ + log_contextual_errno(ctx, __FILE__, __LINE__, __VA_ARGS__) + #define LOG_AND_NOTIFY_ERR(...) \ - log_and_notify(ctx, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) + log_and_notify(conf, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) #define LOG_AND_NOTIFY_WARN(...) \ - log_and_notify(ctx, LOG_CLASS_WARNING, __FILE__, __LINE__, __VA_ARGS__) + log_and_notify(conf, LOG_CLASS_WARNING, __FILE__, __LINE__, __VA_ARGS__) #define LOG_AND_NOTIFY_ERRNO(...) \ - log_errno_and_notify(ctx, LOG_CLASS_ERROR, __FILE__, __LINE__, __VA_ARGS__) + log_and_notify_errno(conf, __FILE__, __LINE__, __VA_ARGS__) static char * get_shell(void) @@ -471,8 +506,7 @@ value_to_wchars(struct context *ctx, wchar_t **res) size_t chars = mbstowcs(NULL, ctx->value, 0); if (chars == (size_t)-1) { - LOG_AND_NOTIFY_ERR("%s:%d: [%s].%s: %s is not a valid string value", - ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); + LOG_CONTEXTUAL_ERR("not a valid string value"); return false; } @@ -486,16 +520,12 @@ value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha) { unsigned long value; if (!value_to_ulong(ctx, 16, &value)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s is not a valid color value", - ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); + LOG_CONTEXTUAL_ERR("not a valid color value"); return false; } if (!allow_alpha && (value & 0xff000000) != 0) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: color value must not have an alpha component", - ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); + LOG_CONTEXTUAL_ERR("color value must not have an alpha component"); return false; } @@ -516,9 +546,7 @@ value_to_two_colors(struct context *ctx, const char *second_as_str = strtok(NULL, " "); if (first_as_str == NULL || second_as_str == NULL) { - LOG_AND_NOTIFY_ERR("%s:%d: [%s].%s: %s: invalid double color value", - ctx->path, ctx->lineno, ctx->section, ctx->key, - ctx->value); + LOG_CONTEXTUAL_ERR("invalid double color value"); goto out; } @@ -550,10 +578,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_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: invalid px value " - "(must be on the form 12px)", - ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); + LOG_CONTEXTUAL_ERR("invalid px value (must be on the form 12px)"); return false; } res->pt = 0; @@ -561,9 +586,7 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) } else { double value; if (!value_to_double(ctx, &value)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: invalid decimal value", - ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); + LOG_CONTEXTUAL_ERR("invalid decimal value"); return false; } res->pt = value; @@ -594,9 +617,8 @@ value_to_fonts(struct context *ctx) struct config_font font_data; if (!config_font_parse(font, &font_data)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: invalid font specification", - ctx->path, ctx->lineno, ctx->section, ctx->key, font); + ctx->value = font; + LOG_CONTEXTUAL_ERR("invalid font specification"); goto err; } @@ -673,9 +695,7 @@ value_to_spawn_template(struct context *ctx, char **argv = NULL; if (!tokenize_cmdline(ctx->value, &argv)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: syntax error in command line", - ctx->path, ctx->lineno, ctx->section, ctx->key, ctx->value); + LOG_CONTEXTUAL_ERR("syntax error in command line"); return false; } @@ -724,9 +744,7 @@ parse_section_main(struct context *ctx) const char *home_dir = get_user_home_dir(); if (home_dir == NULL) { - LOG_AND_NOTIFY_ERRNO( - "%s:%d: [main].include: %s: failed to expand '~'", - path, lineno, value); + LOG_CONTEXTUAL_ERRNO("failed to expand '~'"); return false; } @@ -736,9 +754,7 @@ parse_section_main(struct context *ctx) include_path = value; if (include_path[0] != '/') { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].include: %s: not an absolute path", - path, lineno, include_path); + LOG_CONTEXTUAL_ERR("not an absolute path"); free(_include_path); return false; } @@ -746,9 +762,7 @@ parse_section_main(struct context *ctx) FILE *include = fopen(include_path, "r"); if (include == NULL) { - LOG_AND_NOTIFY_ERRNO( - "%s:%d: [main].include: %s: failed to open", - path, lineno, include_path); + LOG_CONTEXTUAL_ERRNO("failed to open"); free(_include_path); return false; } @@ -792,9 +806,7 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "initial-window-size-pixels") == 0) { unsigned width, height; if (sscanf(value, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].initial-window-size-pixels: %s: invalid size " - "(must be on the form WIDTHxHEIGHT)", path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid size (must be on the form WIDTHxHEIGHT)"); return false; } @@ -806,9 +818,7 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "initial-window-size-chars") == 0) { unsigned width, height; if (sscanf(value, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].initial-window-size-chars: %s: invalid size " - "(must be on the form WIDTHxHEIGHT)", path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid size (must be on the form WIDTHxHEIGHT)"); return false; } @@ -826,10 +836,8 @@ parse_section_main(struct context *ctx) bool invalid_mode = !center && mode[0] != '\0'; if ((ret != 2 && ret != 3) || invalid_mode) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].pad: %s: invalid padding " - "(must be on the form PAD_XxPAD_Y [center])", - path, lineno, value); + LOG_CONTEXTUAL_ERR( + "invalid padding (must be on the form PAD_XxPAD_Y [center])"); return false; } @@ -841,9 +849,7 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "resize-delay-ms") == 0) { unsigned long ms; if (!value_to_ulong(ctx, 10, &ms)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].resize-delay-ms: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } @@ -887,10 +893,7 @@ parse_section_main(struct context *ctx) memset(&conf->bell, 0, sizeof(conf->bell)); } else { - LOG_AND_NOTIFY_ERR( - "%s%d: [main].bell: %s: " - "not one of 'set-urgency', 'notify' or 'none'", - path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'set-urgency', 'notify' or 'none'"); return false; } } @@ -903,10 +906,7 @@ parse_section_main(struct context *ctx) else if (strcmp(value, "fullscreen") == 0) conf->startup_mode = STARTUP_FULLSCREEN; else { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].initial-window-mode: %s: not one of " - "'windows', 'maximized' or 'fullscreen'", - path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'windows', 'maximized' or 'fullscreen'"); return false; } } @@ -966,9 +966,7 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "workers") == 0) { unsigned long count; if (!value_to_ulong(ctx, 10, &count)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].workers: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } conf->render_worker_count = count; @@ -1027,10 +1025,7 @@ parse_section_main(struct context *ctx) } } - LOG_AND_NOTIFY_ERR( - "%s:%d: [main].selection-target: %s: not one of " - "'none', 'primary', 'clipboard' or 'both", - path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'none', 'primary', 'clipboard' or 'both"); return false; } @@ -1043,9 +1038,7 @@ parse_section_main(struct context *ctx) else if (strcmp(value, "always") == 0) conf->url.osc8_underline = OSC8_UNDERLINE_ALWAYS; else { - LOG_AND_NOTIFY_ERR( - "%s:%u: [main].osc8-underline: %s: not one of " - "'url-mode', or 'always'", path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'url-mode', or 'always'"); return false; } } @@ -1054,8 +1047,7 @@ parse_section_main(struct context *ctx) conf->box_drawings_uses_font_glyphs = value_to_bool(ctx); else { - LOG_AND_NOTIFY_ERR("%s:%u: [main].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -1067,8 +1059,6 @@ parse_section_bell(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "urgent") == 0) conf->bell.urgent = value_to_bool(ctx); @@ -1081,8 +1071,7 @@ parse_section_bell(struct context *ctx) else if (strcmp(key, "command-focused") == 0) conf->bell.command_focused = value_to_bool(ctx); else { - LOG_AND_NOTIFY_ERR("%s:%u: [bell].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -1095,15 +1084,11 @@ parse_section_scrollback(struct context *ctx) struct config *conf = ctx->conf; const char *key = ctx->key; const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "lines") == 0) { unsigned long lines; if (!value_to_ulong(ctx, 10, &lines)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [scrollback].lines: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } conf->scrollback.lines = lines; @@ -1117,10 +1102,7 @@ parse_section_scrollback(struct context *ctx) else if (strcmp(value, "relative") == 0) conf->scrollback.indicator.position = SCROLLBACK_INDICATOR_POSITION_RELATIVE; else { - LOG_AND_NOTIFY_ERR( - "%s:%d: [scrollback].indicator-position: %s: not one of " - "'none', 'fixed' or 'relative'", - path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'none', 'fixed' or 'relative'"); return false; } } @@ -1138,9 +1120,7 @@ parse_section_scrollback(struct context *ctx) size_t len = mbstowcs(NULL, value, 0); if (len == (size_t)-1) { - LOG_AND_NOTIFY_ERRNO( - "%s:%d: [scrollback].indicator-format: %s: " - "invalid free form text", path, lineno, value); + LOG_CONTEXTUAL_ERRNO("invalid free form text"); return false; } @@ -1152,9 +1132,7 @@ parse_section_scrollback(struct context *ctx) else if (strcmp(key, "multiplier") == 0) { double multiplier; if (!value_to_double(ctx, &multiplier)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [scrollback].multiplier: %s: invalid decimal value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid decimal value"); return false; } @@ -1162,8 +1140,7 @@ parse_section_scrollback(struct context *ctx) } else { - LOG_AND_NOTIFY_ERR("%s:%u: [scrollback].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -1176,8 +1153,6 @@ parse_section_url(struct context *ctx) struct config *conf = ctx->conf; const char *key = ctx->key; const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "launch") == 0) { if (!value_to_spawn_template(ctx, &conf->url.launch)) @@ -1199,9 +1174,7 @@ parse_section_url(struct context *ctx) else if (strcmp(value, "always") == 0) conf->url.osc8_underline = OSC8_UNDERLINE_ALWAYS; else { - LOG_AND_NOTIFY_ERR( - "%s:%u: [url].osc8-underline: %s: not one of " - "'url-mode', or 'always'", path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'url-mode', or 'always'"); return false; } } @@ -1233,9 +1206,8 @@ parse_section_url(struct context *ctx) size_t chars = mbstowcs(NULL, prot, 0); if (chars == (size_t)-1) { - LOG_AND_NOTIFY_ERRNO( - "%s:%u: [url].protocols: %s: invalid protocol", - path, lineno, prot); + ctx->value = prot; + LOG_CONTEXTUAL_ERRNO("invalid protocol"); return false; } @@ -1273,8 +1245,7 @@ parse_section_url(struct context *ctx) } else { - LOG_AND_NOTIFY_ERR("%s:%d: [url].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -1286,9 +1257,6 @@ parse_section_colors(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; size_t key_len = strlen(key); uint8_t last_digit = (unsigned char)key[key_len - 1] - '0'; @@ -1297,14 +1265,12 @@ parse_section_colors(struct context *ctx) if (isdigit(key[0])) { unsigned long index; if (!str_to_ulong(key, 0, &index)) { - LOG_AND_NOTIFY_ERR("%s:%d: [colors].%s: %s: invalid integer value", - path, lineno, key, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } if (index >= ALEN(conf->colors.table)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [colors].%s: %s: color index outside range 0-%zu", - path, lineno, key, value, ALEN(conf->colors.table)); + LOG_CONTEXTUAL_ERR("color index outside range 0-%zu", + ALEN(conf->colors.table)); return false; } color = &conf->colors.table[index]; @@ -1360,10 +1326,7 @@ parse_section_colors(struct context *ctx) else if (strcmp(key, "alpha") == 0) { double alpha; if (!value_to_double(ctx, &alpha) || alpha < 0. || alpha > 1.) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [colors].alpha: %s: " - "invalid decimal value, or not in range 0.0-1.0", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid decimal value, or not in range 0.0-1.0"); return false; } @@ -1372,8 +1335,7 @@ parse_section_colors(struct context *ctx) } else { - LOG_AND_NOTIFY_ERR("%s:%d: [colors].%s is not valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not valid option"); return false; } @@ -1391,8 +1353,6 @@ parse_section_cursor(struct context *ctx) struct config *conf = ctx->conf; const char *key = ctx->key; const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "style") == 0) { if (strcmp(value, "block") == 0) @@ -1403,9 +1363,7 @@ parse_section_cursor(struct context *ctx) conf->cursor.style = CURSOR_UNDERLINE; else { - LOG_AND_NOTIFY_ERR("%s:%d: [cursor].style: %s: not one of " - "'block', 'beam' or 'underline'", - path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'block', 'beam' or 'underline'"); return false; } } @@ -1438,8 +1396,7 @@ parse_section_cursor(struct context *ctx) } else { - LOG_AND_NOTIFY_ERR("%s:%d: [cursor].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -1451,8 +1408,6 @@ parse_section_mouse(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "hide-when-typing") == 0) conf->mouse.hide_when_typing = value_to_bool(ctx); @@ -1461,8 +1416,7 @@ parse_section_mouse(struct context *ctx) conf->mouse.alternate_scroll_mode = value_to_bool(ctx); else { - LOG_AND_NOTIFY_ERR("%s:%d: [mouse].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -1475,8 +1429,6 @@ parse_section_csd(struct context *ctx) struct config *conf = ctx->conf; const char *key = ctx->key; const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "preferred") == 0) { if (strcmp(value, "server") == 0) @@ -1486,9 +1438,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(value, "none") == 0) conf->csd.preferred = CONF_CSD_PREFER_NONE; else { - LOG_AND_NOTIFY_ERR( - "%s:%d: csd.preferred: %s: not one of " - "'server', 'client' or 'none'", path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'server', 'client' or 'none'"); return false; } } @@ -1514,9 +1464,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "size") == 0) { unsigned long pixels; if (!value_to_ulong(ctx, 10, &pixels)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [csd].size: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } @@ -1526,9 +1474,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "button-width") == 0) { unsigned long pixels; if (!value_to_ulong(ctx, 10, &pixels)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [csd].button-width: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } @@ -1583,9 +1529,7 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "border-width") == 0) { unsigned long width; if (!value_to_ulong(ctx, 10, &width)) { - LOG_AND_NOTIFY_ERR( - "%s:%u: [csd].border-width: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } @@ -1593,8 +1537,7 @@ parse_section_csd(struct context *ctx) } else { - LOG_AND_NOTIFY_ERR("%s:%u: [csd].%s is not a valid action", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid action: %s", key); return false; } @@ -1651,8 +1594,7 @@ parse_modifiers(struct context *ctx, const char *text, size_t len, else if (strcmp(key, XKB_MOD_NAME_LOGO) == 0) modifiers->meta = true; else { - LOG_AND_NOTIFY_ERR("%s:%d: %s: not a valid modifier name", - ctx->path, ctx->lineno, ctx->key); + LOG_CONTEXTUAL_ERR("not a valid modifier name: %s", key); goto out; } } @@ -1690,27 +1632,10 @@ value_to_key_combos(struct context *ctx, struct key_combo_list *key_combos) key++; /* Skip past the '+' */ } -#if 0 - if (modifiers.shift && strlen(key) == 1 && (*key >= 'A' && *key <= 'Z')) { - LOG_WARN( - "%s:%d: [%s].%s: %s: " - "upper case keys not supported with explicit 'Shift' modifier", - path, lineno, section, option, combo); - user_notification_add( - &conf->notifications, USER_NOTIFICATION_DEPRECATED, - "%s:%d: [%s].%s: \033[1m%s\033[m: " - "shifted keys not supported with explicit \033[1mShift\033[m " - "modifier", - path, lineno, section, option, combo); - *key = *key - 'A' + 'a'; - } -#endif /* Translate key name to symbol */ xkb_keysym_t sym = xkb_keysym_from_name(key, 0); if (sym == XKB_KEY_NoSymbol) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: is not a valid XKB key name", - ctx->path, ctx->lineno, ctx->section, ctx->key, key); + LOG_CONTEXTUAL_ERR("not a valid XKB key name: %s", key); goto err; } @@ -1766,8 +1691,8 @@ has_key_binding_collisions(struct context *ctx, if (shift && alt && ctrl && meta && sym) { bool has_pipe = combo1->pipe.argv.args != NULL; - LOG_AND_NOTIFY_ERR("%s:%d: %s already mapped to '%s%s%s%s'", - ctx->path, ctx->lineno, combo2->text, + LOG_CONTEXTUAL_ERR("%s already mapped to '%s%s%s%s'", + combo2->text, action_map[combo1->action], has_pipe ? " [" : "", has_pipe ? combo1->pipe.argv.args[0] : "", @@ -1834,7 +1759,7 @@ pipe_argv_from_value(struct context *ctx, char ***argv) const char *pipe_cmd_end = strrchr(ctx->value, ']'); if (pipe_cmd_end == NULL) { - LOG_AND_NOTIFY_ERR("%s:%d: unclosed '['", ctx->path, ctx->lineno); + LOG_CONTEXTUAL_ERR("unclosed '['"); return -1; } @@ -1842,7 +1767,7 @@ pipe_argv_from_value(struct context *ctx, char ***argv) char *cmd = xstrndup(&ctx->value[1], pipe_len); if (!tokenize_cmdline(cmd, argv)) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error in command line", ctx->path, ctx->lineno); + LOG_CONTEXTUAL_ERR("syntax error in command line"); free(cmd); return -1; } @@ -1965,8 +1890,7 @@ parse_key_binding_section(struct context *ctx, return true; } - LOG_AND_NOTIFY_ERR("%s:%u: [%s].%s is not a valid action", - ctx->path, ctx->lineno, ctx->section, ctx->key); + LOG_CONTEXTUAL_ERR("not a valid action: %s", ctx->key); free(pipe_argv); return false; } @@ -2150,9 +2074,7 @@ value_to_mouse_combos(struct context *ctx, struct key_combo_list *key_combos) if (!parse_modifiers(ctx, combo, key - combo, &modifiers)) goto err; if (modifiers.shift) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: Shift cannot be used in mouse bindings", - ctx->path, ctx->lineno, ctx->section, ctx->key); + LOG_CONTEXTUAL_ERR("Shift cannot be used in mouse bindings"); goto err; } key++; /* Skip past the '+' */ @@ -2170,13 +2092,9 @@ value_to_mouse_combos(struct context *ctx, struct key_combo_list *key_combos) unsigned long value = strtoul(_count, &end, 10); if (_count[0] == '\0' || *end != '\0' || errno != 0) { if (errno != 0) - LOG_AND_NOTIFY_ERRNO( - "%s:%d: [%s].%s: %s: invalid click count", - ctx->path, ctx->lineno, ctx->section, ctx->key, _count); + LOG_CONTEXTUAL_ERRNO("invalid click count: %s", _count); else - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s: invalid click count", - ctx->path, ctx->lineno, ctx->section, ctx->key, _count); + LOG_CONTEXTUAL_ERR("invalid click count: %s", _count); goto err; } count = value; @@ -2206,8 +2124,7 @@ value_to_mouse_combos(struct context *ctx, struct key_combo_list *key_combos) } if (button == 0) { - LOG_AND_NOTIFY_ERR("%s:%d: [%s].%s: %s: invalid mouse button name", - ctx->path, ctx->lineno, ctx->section, ctx->key, key); + LOG_CONTEXTUAL_ERR("invalid mouse button name: %s", key); goto err; } @@ -2265,8 +2182,8 @@ has_mouse_binding_collisions(struct context *ctx, if (shift && alt && ctrl && meta && button && count) { bool has_pipe = combo1->pipe.argv.args != NULL; - LOG_AND_NOTIFY_ERR("%s:%d: %s already mapped to '%s%s%s%s'", - ctx->path, ctx->lineno, combo2->text, + LOG_CONTEXTUAL_ERR("%s already mapped to '%s%s%s%s'", + combo2->text, binding_action_map[combo1->action], has_pipe ? " [" : "", has_pipe ? combo1->pipe.argv.args[0] : "", @@ -2286,8 +2203,6 @@ parse_section_mouse_bindings(struct context *ctx) struct config *conf = ctx->conf; const char *key = ctx->key; const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; char **pipe_argv; @@ -2378,8 +2293,7 @@ parse_section_mouse_bindings(struct context *ctx) return true; } - LOG_AND_NOTIFY_ERR("%s:%u: [mouse-bindings].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); free(pipe_argv); return false; } @@ -2410,10 +2324,8 @@ parse_section_tweak(struct context *ctx) } } - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].scaling-filter: %s: not one of " - "'none', 'nearest', 'bilinear', 'cubic' or 'lanczos3'", - path, lineno, value); + LOG_CONTEXTUAL_ERR( + "not one of 'none', 'nearest', 'bilinear', 'cubic' or 'lanczos3'"); return false; } @@ -2434,19 +2346,15 @@ parse_section_tweak(struct context *ctx) #if !defined(FOOT_GRAPHEME_CLUSTERING) if (conf->tweak.grapheme_shaping) { - LOG_AND_NOTIFY_WARN( - "%s:%d: [tweak].grapheme-shaping: " - "enabled, but foot was not compiled with support for it", - path, lineno); + LOG_CONTEXTUAL_WARN( + "foot was not compiled with support for grapheme shaping"); conf->tweak.grapheme_shaping = false; } #endif if (conf->tweak.grapheme_shaping && !conf->can_shape_grapheme) { LOG_WARN( - "%s:%d [tweak].grapheme-shaping: " - "enabled, but fcft was not compiled with support for it", - path, lineno); + "fcft was not compiled with support for grapheme shaping"); /* Keep it enabled though - this will cause us to do * grapheme-clustering at least */ @@ -2462,9 +2370,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(value, "wcswidth") == 0) conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_WCSWIDTH; else { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].grapheme-width-method: %s: not one of " - "'wcswidth' or 'double-width'", path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'wcswidth' or 'double-width'"); return false; } @@ -2485,9 +2391,7 @@ parse_section_tweak(struct context *ctx) conf->tweak.render_timer_osd = true; conf->tweak.render_timer_log = true; } else { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].render-timer: %s: not one of " - "none', 'osd', 'log' or 'both'", path, lineno, value); + LOG_CONTEXTUAL_ERR("not one of 'none', 'osd', 'log' or 'both'"); return false; } } @@ -2495,16 +2399,12 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "delayed-render-lower") == 0) { unsigned long ns; if (!value_to_ulong(ctx, 10, &ns)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].delayed-render-lower: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } if (ns > 16666666) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].delayed-render-lower: %s: " - "timeout must not exceed 16ms", path, lineno, value); + LOG_CONTEXTUAL_ERR("timeout must not exceed 16ms"); return false; } @@ -2515,16 +2415,12 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "delayed-render-upper") == 0) { unsigned long ns; if (!value_to_ulong(ctx, 10, &ns)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].delayed-render-upper: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } if (ns > 16666666) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].delayed-render-upper: %s: " - "timeout must not exceed 16ms", path, lineno, value); + LOG_CONTEXTUAL_ERR("timeout must not exceed 16ms"); return false; } @@ -2535,9 +2431,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "max-shm-pool-size-mb") == 0) { unsigned long mb; if (!value_to_ulong(ctx, 10, &mb)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].max-shm-pool-size-mb: %s: invalid integer value", - path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid integer value"); return false; } @@ -2549,9 +2443,7 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "box-drawing-base-thickness") == 0) { double base_thickness; if (!value_to_double(ctx, &base_thickness)) { - LOG_AND_NOTIFY_ERR( - "%s:%d: [tweak].box-drawing-base-thickness: %s: " - "invalid decimal value", path, lineno, value); + LOG_CONTEXTUAL_ERR("invalid decimal value"); return false; } @@ -2572,8 +2464,7 @@ parse_section_tweak(struct context *ctx) conf->tweak.font_monospace_warn = value_to_bool(ctx); else { - LOG_AND_NOTIFY_ERR("%s:%u: [tweak].%s is not a valid option", - path, lineno, key); + LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; } @@ -2583,7 +2474,6 @@ 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*/ while (*kv && isspace(*kv)) ++kv; @@ -2762,7 +2652,7 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar if (key_value[0] == '[') { char *end = strchr(key_value, ']'); if (end == NULL) { - LOG_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, context.lineno, key_value); + LOG_CONTEXTUAL_ERR("syntax error: no closing ']'"); error_or_continue(); } @@ -2770,7 +2660,7 @@ parse_config_file(FILE *f, struct config *conf, const char *path, bool errors_ar section = str_to_section(&key_value[1]); if (section == SECTION_COUNT) { - LOG_AND_NOTIFY_ERR("%s:%d: invalid section name: %s", path, context.lineno, &key_value[1]); + LOG_CONTEXTUAL_ERR("invalid section name: %s", &key_value[1]); error_or_continue(); } @@ -2786,7 +2676,7 @@ 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_AND_NOTIFY_ERR("%s:%d: syntax error: %s", path, context.lineno, key_value); + LOG_CONTEXTUAL_ERR("syntax error: key/value pair has no value"); if (errors_are_fatal) goto err; break; @@ -3117,21 +3007,6 @@ config_load(struct config *conf, const char *conf_path, add_default_url_bindings(conf); add_default_mouse_bindings(conf); - /* - * TODO: replace LOG_AND_NOTIFY_*() with custom loggers that - * doesn’t take a context - */ - struct context context = { - .conf = conf, - .section = "", - .key = "", - .value = "", - .path = conf_path, - .lineno = 0, - .errors_are_fatal = errors_are_fatal, - }; - struct context *ctx = &context; - struct config_file conf_file = {.path = NULL, .fd = -1}; if (conf_path != NULL) { int fd = open(conf_path, O_RDONLY); @@ -3220,7 +3095,7 @@ config_override_apply(struct config *conf, config_override_t *overrides, bool er if (!parse_key_value( it->item, &context.section, &context.key, &context.value)) { - LOG_AND_NOTIFY_ERR("syntax error: %s", it->item); + LOG_CONTEXTUAL_ERR("syntax error: key/value pair has no value"); if (errors_are_fatal) return false; continue; @@ -3228,8 +3103,7 @@ config_override_apply(struct config *conf, config_override_t *overrides, bool er enum section section = str_to_section(context.section); if (section == SECTION_COUNT) { - LOG_AND_NOTIFY_ERR( - "override: invalid section name: %s", context.section); + LOG_CONTEXTUAL_ERR("invalid section name: %s", context.section); if (errors_are_fatal) return false; continue; diff --git a/log.h b/log.h index dc458199..e499cf29 100644 --- a/log.h +++ b/log.h @@ -11,7 +11,8 @@ enum log_class { LOG_CLASS_ERROR, LOG_CLASS_WARNING, LOG_CLASS_INFO, - LOG_CLASS_DEBUG + LOG_CLASS_DEBUG, + LOG_CLASS_COUNT, }; void log_init(enum log_colorize colorize, bool do_syslog, From def2d80b0a334b65537c50b3c962de115c01fb32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 17:18:33 +0100 Subject: [PATCH 11/23] config: let value_to_ulong() log errors --- config.c | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/config.c b/config.c index 8ea0d8c0..60a123da 100644 --- a/config.c +++ b/config.c @@ -481,7 +481,12 @@ str_to_ulong(const char *s, int base, unsigned long *res) static bool NOINLINE value_to_ulong(struct context *ctx, int base, unsigned long *res) { - return str_to_ulong(ctx->value, base, res); + if (!str_to_ulong(ctx->value, base, res)) { + LOG_CONTEXTUAL_ERR("invalid integer value"); + return false; + } + + return true; } static bool NOINLINE @@ -519,7 +524,7 @@ static bool NOINLINE value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha) { unsigned long value; - if (!value_to_ulong(ctx, 16, &value)) { + if (!str_to_ulong(ctx->value, 16, &value)) { LOG_CONTEXTUAL_ERR("not a valid color value"); return false; } @@ -848,10 +853,8 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "resize-delay-ms") == 0) { unsigned long ms; - if (!value_to_ulong(ctx, 10, &ms)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &ms)) return false; - } conf->resize_delay_ms = ms; } @@ -965,10 +968,8 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "workers") == 0) { unsigned long count; - if (!value_to_ulong(ctx, 10, &count)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &count)) return false; - } conf->render_worker_count = count; } @@ -1087,10 +1088,8 @@ parse_section_scrollback(struct context *ctx) if (strcmp(key, "lines") == 0) { unsigned long lines; - if (!value_to_ulong(ctx, 10, &lines)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &lines)) return false; - } conf->scrollback.lines = lines; } @@ -1463,20 +1462,16 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "size") == 0) { unsigned long pixels; - if (!value_to_ulong(ctx, 10, &pixels)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &pixels)) return false; - } conf->csd.title_height = pixels; } else if (strcmp(key, "button-width") == 0) { unsigned long pixels; - if (!value_to_ulong(ctx, 10, &pixels)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &pixels)) return false; - } conf->csd.button_width = pixels; } @@ -1528,10 +1523,8 @@ parse_section_csd(struct context *ctx) else if (strcmp(key, "border-width") == 0) { unsigned long width; - if (!value_to_ulong(ctx, 10, &width)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &width)) return false; - } conf->csd.border_width_visible = width; } @@ -2398,10 +2391,8 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "delayed-render-lower") == 0) { unsigned long ns; - if (!value_to_ulong(ctx, 10, &ns)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &ns)) return false; - } if (ns > 16666666) { LOG_CONTEXTUAL_ERR("timeout must not exceed 16ms"); @@ -2414,10 +2405,8 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "delayed-render-upper") == 0) { unsigned long ns; - if (!value_to_ulong(ctx, 10, &ns)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &ns)) return false; - } if (ns > 16666666) { LOG_CONTEXTUAL_ERR("timeout must not exceed 16ms"); @@ -2430,10 +2419,8 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "max-shm-pool-size-mb") == 0) { unsigned long mb; - if (!value_to_ulong(ctx, 10, &mb)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + if (!value_to_ulong(ctx, 10, &mb)) return false; - } conf->tweak.max_shm_pool_size = min(mb * 1024 * 1024, INT32_MAX); LOG_WARN("tweak: max-shm-pool-size=%lld bytes", From 328b53b166b59d2fb5258f464dd45745d6df8011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 17:20:17 +0100 Subject: [PATCH 12/23] config: let value_to_double() log errors --- config.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/config.c b/config.c index 60a123da..f0896158 100644 --- a/config.c +++ b/config.c @@ -501,7 +501,12 @@ value_to_double(struct context *ctx, double *res) char *end = NULL; *res = strtod(s, &end); - return errno == 0 && *end == '\0'; + if (!(errno == 0 && *end == '\0')) { + LOG_CONTEXTUAL_ERR("invalid decimal value"); + return false; + } + + return true; } static bool NOINLINE @@ -590,10 +595,8 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) res->px = value; } else { double value; - if (!value_to_double(ctx, &value)) { - LOG_CONTEXTUAL_ERR("invalid decimal value"); + if (!value_to_double(ctx, &value)) return false; - } res->pt = value; res->px = 0; } @@ -1130,11 +1133,8 @@ parse_section_scrollback(struct context *ctx) else if (strcmp(key, "multiplier") == 0) { double multiplier; - if (!value_to_double(ctx, &multiplier)) { - LOG_CONTEXTUAL_ERR("invalid decimal value"); + if (!value_to_double(ctx, &multiplier)) return false; - } - conf->scrollback.multiplier = multiplier; } @@ -1324,8 +1324,11 @@ parse_section_colors(struct context *ctx) else if (strcmp(key, "alpha") == 0) { double alpha; - if (!value_to_double(ctx, &alpha) || alpha < 0. || alpha > 1.) { - LOG_CONTEXTUAL_ERR("invalid decimal value, or not in range 0.0-1.0"); + if (!value_to_double(ctx, &alpha)) + return false; + + if (alpha < 0. || alpha > 1.) { + LOG_CONTEXTUAL_ERR("not in range 0.0-1.0"); return false; } @@ -2429,10 +2432,8 @@ parse_section_tweak(struct context *ctx) else if (strcmp(key, "box-drawing-base-thickness") == 0) { double base_thickness; - if (!value_to_double(ctx, &base_thickness)) { - LOG_CONTEXTUAL_ERR("invalid decimal value"); + if (!value_to_double(ctx, &base_thickness)) return false; - } conf->tweak.box_drawing_base_thickness = base_thickness; LOG_WARN("tweak: box-drawing-base-thickness=%f", From 57c7fb33af86754b5a15afa5786c3fdc702859cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 5 Nov 2021 18:11:53 +0100 Subject: [PATCH 13/23] config: add value_to_enum() --- config.c | 212 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 108 insertions(+), 104 deletions(-) diff --git a/config.c b/config.c index f0896158..dfb8a2db 100644 --- a/config.c +++ b/config.c @@ -525,6 +525,32 @@ value_to_wchars(struct context *ctx, wchar_t **res) return true; } +static int NOINLINE +value_to_enum(struct context *ctx, const char **value_map) +{ + size_t str_len = 0; + size_t count = 0; + + for (; value_map[count] != NULL; count++) { + if (strcasecmp(value_map[count], ctx->value) == 0) + return count; + str_len += strlen(value_map[count]); + } + + const size_t size = str_len + count * 4 + 1; + char *valid_values = xmalloc(size); + int idx = 0; + + for (size_t i = 0; i < count; i++) + idx += snprintf(&valid_values[idx], size - idx, "'%s', ", value_map[i]); + + if (count > 0) + valid_values[idx - 2] = '\0'; + + LOG_CONTEXTUAL_ERR("not one of %s", valid_values); + return -1; +} + static bool NOINLINE value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha) { @@ -887,34 +913,29 @@ parse_section_main(struct context *ctx) }; tll_push_back(conf->notifications, deprecation); - if (strcmp(value, "set-urgency") == 0) { - memset(&conf->bell, 0, sizeof(conf->bell)); - conf->bell.urgent = true; - } - else if (strcmp(value, "notify") == 0) { - memset(&conf->bell, 0, sizeof(conf->bell)); - conf->bell.notify = true; - } - else if (strcmp(value, "none") == 0) { - memset(&conf->bell, 0, sizeof(conf->bell)); - } - else { - LOG_CONTEXTUAL_ERR("not one of 'set-urgency', 'notify' or 'none'"); + int bell = value_to_enum( + ctx, (const char *[]){"set-urgency", "notify", "none", NULL}); + + switch (bell) { + case 0: conf->bell.urgent = true; break; + case 1: conf->bell.notify = true; break; + case 2: memset(&conf->bell, 0, sizeof(conf->bell)); break; + + case -1: + default: return false; + } } else if (strcmp(key, "initial-window-mode") == 0) { - if (strcmp(value, "windowed") == 0) - conf->startup_mode = STARTUP_WINDOWED; - else if (strcmp(value, "maximized") == 0) - conf->startup_mode = STARTUP_MAXIMIZED; - else if (strcmp(value, "fullscreen") == 0) - conf->startup_mode = STARTUP_FULLSCREEN; - else { - LOG_CONTEXTUAL_ERR("not one of 'windows', 'maximized' or 'fullscreen'"); + int mode = value_to_enum( + ctx, (const char *[]){"windowed", "maximized", "fullscreen", NULL}); + + if (mode < 0) return false; - } + + conf->startup_mode = mode; } else if (strcmp(key, "font") == 0 || @@ -1015,36 +1036,26 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "selection-target") == 0) { - static const char values[][12] = { - [SELECTION_TARGET_NONE] = "none", - [SELECTION_TARGET_PRIMARY] = "primary", - [SELECTION_TARGET_CLIPBOARD] = "clipboard", - [SELECTION_TARGET_BOTH] = "both", - }; + int target = value_to_enum( + ctx, (const char *[]){"none", "primary", "clipboard", "both", NULL}); - for (size_t i = 0; i < ALEN(values); i++) { - if (strcasecmp(value, values[i]) == 0) { - conf->selection_target = i; - return true; - } - } + if (target < 0) + return false; - LOG_CONTEXTUAL_ERR("not one of 'none', 'primary', 'clipboard' or 'both"); - return false; + conf->selection_target = target; } else if (strcmp(key, "osc8-underline") == 0) { deprecated_url_option( conf, "osc8-underline", "osc8-underline", path, lineno); - if (strcmp(value, "url-mode") == 0) - conf->url.osc8_underline = OSC8_UNDERLINE_URL_MODE; - else if (strcmp(value, "always") == 0) - conf->url.osc8_underline = OSC8_UNDERLINE_ALWAYS; - else { - LOG_CONTEXTUAL_ERR("not one of 'url-mode', or 'always'"); + int mode = value_to_enum( + ctx, (const char *[]){"url-mode", "always", NULL}); + + if (mode < 0) return false; - } + + conf->url.osc8_underline = mode; } else if (strcmp(key, "box-drawings-uses-font-glyphs") == 0) @@ -1097,16 +1108,13 @@ parse_section_scrollback(struct context *ctx) } else if (strcmp(key, "indicator-position") == 0) { - if (strcmp(value, "none") == 0) - conf->scrollback.indicator.position = SCROLLBACK_INDICATOR_POSITION_NONE; - else if (strcmp(value, "fixed") == 0) - conf->scrollback.indicator.position = SCROLLBACK_INDICATOR_POSITION_FIXED; - else if (strcmp(value, "relative") == 0) - conf->scrollback.indicator.position = SCROLLBACK_INDICATOR_POSITION_RELATIVE; - else { - LOG_CONTEXTUAL_ERR("not one of 'none', 'fixed' or 'relative'"); + int position = value_to_enum( + ctx, (const char *[]){"none", "fixed", "relative", NULL}); + + if (position < 0) return false; - } + + conf->scrollback.indicator.position = position; } else if (strcmp(key, "indicator-format") == 0) { @@ -1168,14 +1176,13 @@ parse_section_url(struct context *ctx) } else if (strcmp(key, "osc8-underline") == 0) { - if (strcmp(value, "url-mode") == 0) - conf->url.osc8_underline = OSC8_UNDERLINE_URL_MODE; - else if (strcmp(value, "always") == 0) - conf->url.osc8_underline = OSC8_UNDERLINE_ALWAYS; - else { - LOG_CONTEXTUAL_ERR("not one of 'url-mode', or 'always'"); + int mode = value_to_enum( + ctx, (const char *[]){"url-mode", "always", NULL}); + + if (mode < 0) return false; - } + + conf->url.osc8_underline = mode; } else if (strcmp(key, "protocols") == 0) { @@ -1354,20 +1361,15 @@ parse_section_cursor(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *value = ctx->value; if (strcmp(key, "style") == 0) { - if (strcmp(value, "block") == 0) - conf->cursor.style = CURSOR_BLOCK; - else if (strcmp(value, "beam") == 0 || strcmp(value, "bar") == 0) - conf->cursor.style = CURSOR_BEAM; - else if (strcmp(value, "underline") == 0) - conf->cursor.style = CURSOR_UNDERLINE; + enum cursor_style style = value_to_enum( + ctx, (const char *[]){"block", "underline", "beam", NULL}); - else { - LOG_CONTEXTUAL_ERR("not one of 'block', 'beam' or 'underline'"); + if (style < 0) return false; - } + + conf->cursor.style = style; } else if (strcmp(key, "blink") == 0) @@ -1430,19 +1432,15 @@ parse_section_csd(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *value = ctx->value; if (strcmp(key, "preferred") == 0) { - if (strcmp(value, "server") == 0) - conf->csd.preferred = CONF_CSD_PREFER_SERVER; - else if (strcmp(value, "client") == 0) - conf->csd.preferred = CONF_CSD_PREFER_CLIENT; - else if (strcmp(value, "none") == 0) - conf->csd.preferred = CONF_CSD_PREFER_NONE; - else { - LOG_CONTEXTUAL_ERR("not one of 'server', 'client' or 'none'"); + int csd = value_to_enum( + ctx, (const char *[]){"none", "server", "client", NULL}); + + if (csd < 0) return false; - } + + conf->csd.preferred = csd; } else if (strcmp(key, "font") == 0) { @@ -2304,25 +2302,21 @@ parse_section_tweak(struct context *ctx) unsigned lineno = ctx->lineno; if (strcmp(key, "scaling-filter") == 0) { - static const char filters[][12] = { + static const char *filters[] = { [FCFT_SCALING_FILTER_NONE] = "none", [FCFT_SCALING_FILTER_NEAREST] = "nearest", [FCFT_SCALING_FILTER_BILINEAR] = "bilinear", [FCFT_SCALING_FILTER_CUBIC] = "cubic", [FCFT_SCALING_FILTER_LANCZOS3] = "lanczos3", + NULL, }; - for (size_t i = 0; i < ALEN(filters); i++) { - if (strcmp(value, filters[i]) == 0) { - conf->tweak.fcft_filter = i; - LOG_WARN("tweak: scaling-filter=%s", filters[i]); - return true; - } - } + int filter = value_to_enum(ctx, filters); + if (filter < 0) + return false; - LOG_CONTEXTUAL_ERR( - "not one of 'none', 'nearest', 'bilinear', 'cubic' or 'lanczos3'"); - return false; + conf->tweak.fcft_filter = filter; + LOG_WARN("tweak: scaling-filter=%s", filters[filter]); } else if (strcmp(key, "overflowing-glyphs") == 0) { @@ -2361,33 +2355,43 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "grapheme-width-method") == 0) { - if (strcmp(value, "double-width") == 0) - conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_DOUBLE; - else if (strcmp(value, "wcswidth") == 0) - conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_WCSWIDTH; - else { - LOG_CONTEXTUAL_ERR("not one of 'wcswidth' or 'double-width'"); - return false; - } + int method = value_to_enum( + ctx, (const char *[]){"wcswidth", "double-width", NULL}); + if (method < 0) + return false; + + conf->tweak.grapheme_width_method = method; LOG_WARN("%s:%d [tweak].grapheme-width-method=%s", path, lineno, value); } else if (strcmp(key, "render-timer") == 0) { - if (strcmp(value, "none") == 0) { + int mode = value_to_enum( + ctx, (const char *[]){"none", "osd", "log", "both", NULL}); + + switch (mode) { + case 0: conf->tweak.render_timer_osd = false; conf->tweak.render_timer_log = false; - } else if (strcmp(value, "osd") == 0) { + break; + + case 1: conf->tweak.render_timer_osd = true; conf->tweak.render_timer_log = false; - } else if (strcmp(value, "log") == 0) { + break; + + case 2: conf->tweak.render_timer_osd = false; conf->tweak.render_timer_log = true; - } else if (strcmp(value, "both") == 0) { + break; + + case 3: conf->tweak.render_timer_osd = true; conf->tweak.render_timer_log = true; - } else { - LOG_CONTEXTUAL_ERR("not one of 'none', 'osd', 'log' or 'both'"); + break; + + case -1: + default: return false; } } From c2127fb2de37fe19cd7953327c50aaea66f830bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 11:25:55 +0100 Subject: [PATCH 14/23] config: value_to_bool: return success/fail Write boolean value to a pointer passed as argument, return conversion success/failure --- config.c | 75 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/config.c b/config.c index dfb8a2db..09d684dc 100644 --- a/config.c +++ b/config.c @@ -456,15 +456,30 @@ str_has_prefix(const char *str, const char *prefix) } static bool NOINLINE -value_to_bool(struct context *ctx) +value_to_bool(struct context *ctx, bool *res) { - const char *s = ctx->value; - return strcasecmp(s, "on") == 0 || - strcasecmp(s, "true") == 0 || - strcasecmp(s, "yes") == 0 || - strtoul(s, NULL, 0) > 0; + static const char *const yes[] = {"on", "true", "yes", "1"}; + static const char *const no[] = {"off", "false", "no", "0"}; + + for (size_t i = 0; i < ALEN(yes); i++) { + if (strcasecmp(ctx->value, yes[i]) == 0) { + *res = true; + return true; + } + } + + for (size_t i = 0; i < ALEN(no); i++) { + if (strcasecmp(ctx->value, no[i]) == 0) { + *res = false; + return true; + } + } + + LOG_CONTEXTUAL_ERR("invalid boolean value"); + return false; } + static bool str_to_ulong(const char *s, int base, unsigned long *res) { @@ -821,7 +836,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "login-shell") == 0) { - conf->login_shell = value_to_bool(ctx); + return value_to_bool(ctx, &conf->login_shell); } else if (strcmp(key, "title") == 0) { @@ -830,7 +845,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "locked-title") == 0) - conf->locked_title = value_to_bool(ctx); + return value_to_bool(ctx, &conf->locked_title); else if (strcmp(key, "app-id") == 0) { free(conf->app_id); @@ -893,7 +908,8 @@ parse_section_main(struct context *ctx) conf->bold_in_bright.enabled = true; conf->bold_in_bright.palette_based = true; } else { - conf->bold_in_bright.enabled = value_to_bool(ctx); + if (!value_to_bool(ctx, &conf->bold_in_bright.enabled)) + return false; conf->bold_in_bright.palette_based = false; } } @@ -986,8 +1002,12 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "dpi-aware") == 0) { if (strcmp(value, "auto") == 0) conf->dpi_aware = DPI_AWARE_AUTO; - else - conf->dpi_aware = value_to_bool(ctx) ? DPI_AWARE_YES : DPI_AWARE_NO; + else { + bool value; + if (!value_to_bool(ctx, &value)) + return false; + conf->dpi_aware = value ? DPI_AWARE_YES : DPI_AWARE_NO; + } } else if (strcmp(key, "workers") == 0) { @@ -1024,7 +1044,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "notify-focus-inhibit") == 0) { - conf->notify_focus_inhibit = value_to_bool(ctx); + return value_to_bool(ctx, &conf->notify_focus_inhibit); } else if (strcmp(key, "url-launch") == 0) { @@ -1059,7 +1079,7 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "box-drawings-uses-font-glyphs") == 0) - conf->box_drawings_uses_font_glyphs = value_to_bool(ctx); + return value_to_bool(ctx, &conf->box_drawings_uses_font_glyphs); else { LOG_CONTEXTUAL_ERR("not a valid option: %s", key); @@ -1076,15 +1096,14 @@ parse_section_bell(struct context *ctx) const char *key = ctx->key; if (strcmp(key, "urgent") == 0) - conf->bell.urgent = value_to_bool(ctx); + return value_to_bool(ctx, &conf->bell.urgent); else if (strcmp(key, "notify") == 0) - conf->bell.notify = value_to_bool(ctx); + return value_to_bool(ctx, &conf->bell.notify); else if (strcmp(key, "command") == 0) { if (!value_to_spawn_template(ctx, &conf->bell.command)) return false; - } - else if (strcmp(key, "command-focused") == 0) - conf->bell.command_focused = value_to_bool(ctx); + } else if (strcmp(key, "command-focused") == 0) + return value_to_bool(ctx, &conf->bell.command_focused); else { LOG_CONTEXTUAL_ERR("not a valid option: %s", key); return false; @@ -1373,7 +1392,7 @@ parse_section_cursor(struct context *ctx) } else if (strcmp(key, "blink") == 0) - conf->cursor.blink = value_to_bool(ctx); + return value_to_bool(ctx, &conf->cursor.blink); else if (strcmp(key, "color") == 0) { if (!value_to_two_colors( @@ -1414,10 +1433,10 @@ parse_section_mouse(struct context *ctx) const char *key = ctx->key; if (strcmp(key, "hide-when-typing") == 0) - conf->mouse.hide_when_typing = value_to_bool(ctx); + return value_to_bool(ctx, &conf->mouse.hide_when_typing); else if (strcmp(key, "alternate-scroll-mode") == 0) - conf->mouse.alternate_scroll_mode = value_to_bool(ctx); + return value_to_bool(ctx, &conf->mouse.alternate_scroll_mode); else { LOG_CONTEXTUAL_ERR("not a valid option: %s", key); @@ -2320,19 +2339,22 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "overflowing-glyphs") == 0) { - conf->tweak.overflowing_glyphs = value_to_bool(ctx); + if (!value_to_bool(ctx, &conf->tweak.overflowing_glyphs)) + return false; if (!conf->tweak.overflowing_glyphs) LOG_WARN("tweak: disabled overflowing glyphs"); } else if (strcmp(key, "damage-whole-window") == 0) { - conf->tweak.damage_whole_window = value_to_bool(ctx); + if (!value_to_bool(ctx, &conf->tweak.damage_whole_window)) + return false; if (conf->tweak.damage_whole_window) LOG_WARN("tweak: damage whole window"); } else if (strcmp(key, "grapheme-shaping") == 0) { - conf->tweak.grapheme_shaping = value_to_bool(ctx); + if (!value_to_bool(ctx, &conf->tweak.grapheme_shaping)) + return false; #if !defined(FOOT_GRAPHEME_CLUSTERING) if (conf->tweak.grapheme_shaping) { @@ -2445,7 +2467,8 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "box-drawing-solid-shades") == 0) { - conf->tweak.box_drawing_solid_shades = value_to_bool(ctx); + if (!value_to_bool(ctx, &conf->tweak.box_drawing_solid_shades)) + return false; if (!conf->tweak.box_drawing_solid_shades) LOG_WARN("tweak: box-drawing-solid-shades=%s", @@ -2453,7 +2476,7 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "font-monospace-warn") == 0) - conf->tweak.font_monospace_warn = value_to_bool(ctx); + return value_to_bool(ctx, &conf->tweak.font_monospace_warn); else { LOG_CONTEXTUAL_ERR("not a valid option: %s", key); From 3b27a665da60837a455ca0b39e99625ad85ade75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 11:32:11 +0100 Subject: [PATCH 15/23] config: add value_to_str() Takes a pointer to a char*, free:s it, and strdups the current value. --- config.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/config.c b/config.c index 09d684dc..7d12ac91 100644 --- a/config.c +++ b/config.c @@ -524,6 +524,14 @@ value_to_double(struct context *ctx, double *res) return true; } +static bool NOINLINE +value_to_str(struct context *ctx, char **res) +{ + free(*res); + *res = xstrdup(ctx->value); + return true; +} + static bool NOINLINE value_to_wchars(struct context *ctx, wchar_t **res) { @@ -825,32 +833,23 @@ parse_section_main(struct context *ctx) return ret; } - else if (strcmp(key, "term") == 0) { - free(conf->term); - conf->term = xstrdup(value); - } + else if (strcmp(key, "term") == 0) + return value_to_str(ctx, &conf->term); - else if (strcmp(key, "shell") == 0) { - free(conf->shell); - conf->shell = xstrdup(value); - } + else if (strcmp(key, "shell") == 0) + return value_to_str(ctx, &conf->shell); - else if (strcmp(key, "login-shell") == 0) { + else if (strcmp(key, "login-shell") == 0) return value_to_bool(ctx, &conf->login_shell); - } - else if (strcmp(key, "title") == 0) { - free(conf->title); - conf->title = xstrdup(value); - } + else if (strcmp(key, "title") == 0) + return value_to_str(ctx, &conf->title); else if (strcmp(key, "locked-title") == 0) return value_to_bool(ctx, &conf->locked_title); - else if (strcmp(key, "app-id") == 0) { - free(conf->app_id); - conf->app_id = xstrdup(value); - } + else if (strcmp(key, "app-id") == 0) + return value_to_str(ctx, &conf->app_id); else if (strcmp(key, "initial-window-size-pixels") == 0) { unsigned width, height; From d29c3cf7b76c6a1935684124b4d7cad69de94c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 12:01:57 +0100 Subject: [PATCH 16/23] config: add {str,value}_to_uint{16,32}() This allows us to pass the final pointer directly to the conversion functions, as well as allowing us to print outside-range errors. --- config.c | 172 ++++++++++++++++++++++++----------------------------- config.h | 20 +++---- render.c | 2 +- terminal.c | 2 +- terminal.h | 4 +- 5 files changed, 93 insertions(+), 107 deletions(-) diff --git a/config.c b/config.c index 7d12ac91..e6ae1c14 100644 --- a/config.c +++ b/config.c @@ -480,7 +480,7 @@ value_to_bool(struct context *ctx, bool *res) } -static bool +static bool NOINLINE str_to_ulong(const char *s, int base, unsigned long *res) { if (s == NULL) @@ -494,18 +494,51 @@ str_to_ulong(const char *s, int base, unsigned long *res) } static bool NOINLINE -value_to_ulong(struct context *ctx, int base, unsigned long *res) +str_to_uint32(const char *s, int base, uint32_t *res) { - if (!str_to_ulong(ctx->value, base, res)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); + unsigned long v; + bool ret = str_to_ulong(s, base, &v); + if (v > UINT32_MAX) + return false; + *res = v; + return ret; +} + +static bool NOINLINE +str_to_uint16(const char *s, int base, uint16_t *res) +{ + unsigned long v; + bool ret = str_to_ulong(s, base, &v); + if (v > UINT16_MAX) + return false; + *res = v; + return ret; +} + +static bool NOINLINE +value_to_uint16(struct context *ctx, int base, uint16_t *res) +{ + if (!str_to_uint16(ctx->value, base, res)) { + LOG_CONTEXTUAL_ERR( + "invalid integer value, or outside range 0-%u", UINT16_MAX); return false; } - return true; } static bool NOINLINE -value_to_double(struct context *ctx, double *res) +value_to_uint32(struct context *ctx, int base, uint32_t *res) +{ + if (!str_to_uint32(ctx->value, base, res)){ + LOG_CONTEXTUAL_ERR( + "invalid integer value, or outside range 0-%u", UINT32_MAX); + return false; + } + return true; +} + +static bool NOINLINE +value_to_double(struct context *ctx, float *res) { const char *s = ctx->value; @@ -515,7 +548,7 @@ value_to_double(struct context *ctx, double *res) errno = 0; char *end = NULL; - *res = strtod(s, &end); + *res = strtof(s, &end); if (!(errno == 0 && *end == '\0')) { LOG_CONTEXTUAL_ERR("invalid decimal value"); return false; @@ -577,18 +610,16 @@ value_to_enum(struct context *ctx, const char **value_map) static bool NOINLINE value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha) { - unsigned long value; - if (!str_to_ulong(ctx->value, 16, &value)) { + if (!str_to_uint32(ctx->value, 16, color)) { LOG_CONTEXTUAL_ERR("not a valid color value"); return false; } - if (!allow_alpha && (value & 0xff000000) != 0) { + if (!allow_alpha && (*color & 0xff000000) != 0) { LOG_CONTEXTUAL_ERR("color value must not have an alpha component"); return false; } - *color = value; return true; } @@ -643,7 +674,7 @@ value_to_pt_or_px(struct context *ctx, struct pt_or_px *res) res->pt = 0; res->px = value; } else { - double value; + float value; if (!value_to_double(ctx, &value)) return false; res->pt = value; @@ -894,13 +925,8 @@ parse_section_main(struct context *ctx) conf->center = center; } - else if (strcmp(key, "resize-delay-ms") == 0) { - unsigned long ms; - if (!value_to_ulong(ctx, 10, &ms)) - return false; - - conf->resize_delay_ms = ms; - } + else if (strcmp(key, "resize-delay-ms") == 0) + return value_to_uint16(ctx, 10, &conf->resize_delay_ms); else if (strcmp(key, "bold-text-in-bright") == 0) { if (strcmp(value, "palette-based") == 0) { @@ -1009,12 +1035,8 @@ parse_section_main(struct context *ctx) } } - else if (strcmp(key, "workers") == 0) { - unsigned long count; - if (!value_to_ulong(ctx, 10, &count)) - return false; - conf->render_worker_count = count; - } + else if (strcmp(key, "workers") == 0) + return value_to_uint16(ctx, 10, &conf->render_worker_count); else if (strcmp(key, "word-delimiters") == 0) { wchar_t *word_delimiters; @@ -1118,12 +1140,8 @@ parse_section_scrollback(struct context *ctx) const char *key = ctx->key; const char *value = ctx->value; - if (strcmp(key, "lines") == 0) { - unsigned long lines; - if (!value_to_ulong(ctx, 10, &lines)) - return false; - conf->scrollback.lines = lines; - } + if (strcmp(key, "lines") == 0) + value_to_uint32(ctx, 10, &conf->scrollback.lines); else if (strcmp(key, "indicator-position") == 0) { int position = value_to_enum( @@ -1157,12 +1175,8 @@ parse_section_scrollback(struct context *ctx) } } - else if (strcmp(key, "multiplier") == 0) { - double multiplier; - if (!value_to_double(ctx, &multiplier)) - return false; - conf->scrollback.multiplier = multiplier; - } + else if (strcmp(key, "multiplier") == 0) + return value_to_double(ctx, &conf->scrollback.multiplier); else { LOG_CONTEXTUAL_ERR("not a valid option: %s", key); @@ -1288,13 +1302,12 @@ parse_section_colors(struct context *ctx) if (isdigit(key[0])) { unsigned long index; - if (!str_to_ulong(key, 0, &index)) { - LOG_CONTEXTUAL_ERR("invalid integer value"); - return false; - } - if (index >= ALEN(conf->colors.table)) { - LOG_CONTEXTUAL_ERR("color index outside range 0-%zu", - ALEN(conf->colors.table)); + if (!str_to_ulong(key, 0, &index) || + index >= ALEN(conf->colors.table)) + { + LOG_CONTEXTUAL_ERR( + "invalid color palette index: %s (not in range 0-%zu)", + key, ALEN(conf->colors.table)); return false; } color = &conf->colors.table[index]; @@ -1348,7 +1361,7 @@ parse_section_colors(struct context *ctx) } else if (strcmp(key, "alpha") == 0) { - double alpha; + float alpha; if (!value_to_double(ctx, &alpha)) return false; @@ -1479,74 +1492,49 @@ parse_section_csd(struct context *ctx) conf->csd.color.title = color; } - else if (strcmp(key, "size") == 0) { - unsigned long pixels; - if (!value_to_ulong(ctx, 10, &pixels)) - return false; + else if (strcmp(key, "size") == 0) + return value_to_uint16(ctx, 10, &conf->csd.title_height); - conf->csd.title_height = pixels; - } - - else if (strcmp(key, "button-width") == 0) { - unsigned long pixels; - if (!value_to_ulong(ctx, 10, &pixels)) - return false; - - conf->csd.button_width = pixels; - } + else if (strcmp(key, "button-width") == 0) + return value_to_uint16(ctx, 10, &conf->csd.button_width); else if (strcmp(key, "button-color") == 0) { - uint32_t color; - if (!value_to_color(ctx, &color, true)) + if (!value_to_color(ctx, &conf->csd.color.buttons, true)) return false; conf->csd.color.buttons_set = true; - conf->csd.color.buttons = color; } else if (strcmp(key, "button-minimize-color") == 0) { - uint32_t color; - if (!value_to_color(ctx, &color, true)) + if (!value_to_color(ctx, &conf->csd.color.minimize, true)) return false; conf->csd.color.minimize_set = true; - conf->csd.color.minimize = color; } else if (strcmp(key, "button-maximize-color") == 0) { - uint32_t color; - if (!value_to_color(ctx, &color, true)) + if (!value_to_color(ctx, &conf->csd.color.maximize, true)) return false; conf->csd.color.maximize_set = true; - conf->csd.color.maximize = color; } else if (strcmp(key, "button-close-color") == 0) { - uint32_t color; - if (!value_to_color(ctx, &color, true)) + if (!value_to_color(ctx, &conf->csd.color.close, true)) return false; conf->csd.color.close_set = true; - conf->csd.color.close = color; } else if (strcmp(key, "border-color") == 0) { - uint32_t color; - if (!value_to_color(ctx, &color, true)) + if (!value_to_color(ctx, &conf->csd.color.border, true)) return false; conf->csd.color.border_set = true; - conf->csd.color.border = color; } - else if (strcmp(key, "border-width") == 0) { - unsigned long width; - if (!value_to_ulong(ctx, 10, &width)) - return false; - - conf->csd.border_width_visible = width; - } + else if (strcmp(key, "border-width") == 0) + return value_to_uint16(ctx, 10, &conf->csd.border_width_visible); else { LOG_CONTEXTUAL_ERR("not a valid action: %s", key); @@ -2418,8 +2406,8 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "delayed-render-lower") == 0) { - unsigned long ns; - if (!value_to_ulong(ctx, 10, &ns)) + uint32_t ns; + if (!value_to_uint32(ctx, 10, &ns)) return false; if (ns > 16666666) { @@ -2428,12 +2416,12 @@ parse_section_tweak(struct context *ctx) } conf->tweak.delayed_render_lower_ns = ns; - LOG_WARN("tweak: delayed-render-lower=%lu", ns); + LOG_WARN("tweak: delayed-render-lower=%u", ns); } else if (strcmp(key, "delayed-render-upper") == 0) { - unsigned long ns; - if (!value_to_ulong(ctx, 10, &ns)) + uint32_t ns; + if (!value_to_uint32(ctx, 10, &ns)) return false; if (ns > 16666666) { @@ -2442,25 +2430,23 @@ parse_section_tweak(struct context *ctx) } conf->tweak.delayed_render_upper_ns = ns; - LOG_WARN("tweak: delayed-render-upper=%lu", ns); + LOG_WARN("tweak: delayed-render-upper=%u", ns); } else if (strcmp(key, "max-shm-pool-size-mb") == 0) { - unsigned long mb; - if (!value_to_ulong(ctx, 10, &mb)) + uint32_t mb; + if (!value_to_uint32(ctx, 10, &mb)) return false; - conf->tweak.max_shm_pool_size = min(mb * 1024 * 1024, INT32_MAX); + conf->tweak.max_shm_pool_size = min((int32_t)mb * 1024 * 1024, INT32_MAX); LOG_WARN("tweak: max-shm-pool-size=%lld bytes", (long long)conf->tweak.max_shm_pool_size); } else if (strcmp(key, "box-drawing-base-thickness") == 0) { - double base_thickness; - if (!value_to_double(ctx, &base_thickness)) + if (!value_to_double(ctx, &conf->tweak.box_drawing_base_thickness)) return false; - conf->tweak.box_drawing_base_thickness = base_thickness; LOG_WARN("tweak: box-drawing-base-thickness=%f", conf->tweak.box_drawing_base_thickness); } diff --git a/config.h b/config.h index b47c3537..4b23e465 100644 --- a/config.h +++ b/config.h @@ -19,7 +19,7 @@ enum conf_size_type {CONF_SIZE_PX, CONF_SIZE_CELLS}; struct config_font { char *pattern; - double pt_size; + float pt_size; int px_size; }; DEFINE_LIST(struct config_font); @@ -115,7 +115,7 @@ struct config { } bell; struct { - int lines; + uint32_t lines; struct { enum { @@ -132,7 +132,7 @@ struct config { wchar_t *text; } indicator; - double multiplier; + float multiplier; } scrollback; struct { @@ -212,10 +212,10 @@ struct config { struct { enum { CONF_CSD_PREFER_NONE, CONF_CSD_PREFER_SERVER, CONF_CSD_PREFER_CLIENT } preferred; - int title_height; - int border_width; - int border_width_visible; - int button_width; + uint16_t title_height; + uint16_t border_width; + uint16_t border_width_visible; + uint16_t button_width; struct { bool title_set:1; @@ -235,7 +235,7 @@ struct config { struct config_font_list font; } csd; - size_t render_worker_count; + uint16_t render_worker_count; char *server_socket_path; bool presentation_timings; bool hold_at_exit; @@ -257,8 +257,8 @@ struct config { bool render_timer_osd; bool render_timer_log; bool damage_whole_window; - uint64_t delayed_render_lower_ns; - uint64_t delayed_render_upper_ns; + uint32_t delayed_render_lower_ns; + uint32_t delayed_render_upper_ns; off_t max_shm_pool_size; float box_drawing_base_thickness; bool box_drawing_solid_shades; diff --git a/render.c b/render.c index f232c0be..39957de8 100644 --- a/render.c +++ b/render.c @@ -3538,7 +3538,7 @@ maybe_resize(struct terminal *term, int width, int height, bool force) term->height = height; term->scale = scale; - const int scrollback_lines = term->render.scrollback_lines; + const uint32_t scrollback_lines = term->render.scrollback_lines; /* Screen rows/cols before resize */ const int old_cols = term->cols; diff --git a/terminal.c b/terminal.c index 910a8857..55c29f40 100644 --- a/terminal.c +++ b/terminal.c @@ -580,7 +580,7 @@ fdm_title_update_timeout(struct fdm *fdm, int fd, int events, void *data) static bool initialize_render_workers(struct terminal *term) { - LOG_INFO("using %zu rendering threads", term->render.workers.count); + LOG_INFO("using %hu rendering threads", term->render.workers.count); if (sem_init(&term->render.workers.start, 0, 0) < 0 || sem_init(&term->render.workers.done, 0, 0) < 0) diff --git a/terminal.h b/terminal.h index 8c847cfd..10be6ae3 100644 --- a/terminal.h +++ b/terminal.h @@ -526,7 +526,7 @@ struct terminal { int timer_fd; } title; - int scrollback_lines; /* Number of scrollback lines, from conf (TODO: move out from render struct?) */ + uint32_t scrollback_lines; /* Number of scrollback lines, from conf (TODO: move out from render struct?) */ struct { bool enabled; @@ -535,7 +535,7 @@ struct terminal { /* Render threads + synchronization primitives */ struct { - size_t count; + uint16_t count; sem_t start; sem_t done; mtx_t lock; From 0c0a78498f4207ca9816cd41c5badaf516230072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 12:32:20 +0100 Subject: [PATCH 17/23] config: add value_to_dimensions() --- config.c | 25 +++++++++++++------------ config.h | 4 ++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/config.c b/config.c index e6ae1c14..fd646bb7 100644 --- a/config.c +++ b/config.c @@ -537,6 +537,17 @@ value_to_uint32(struct context *ctx, int base, uint32_t *res) return true; } +static bool NOINLINE +value_to_dimensions(struct context *ctx, uint32_t *x, uint32_t *y) +{ + if (sscanf(ctx->value, "%ux%u", x, y) != 2) { + LOG_CONTEXTUAL_ERR("invalid dimensions (must be on the form AxB)"); + return false; + } + + return true; +} + static bool NOINLINE value_to_double(struct context *ctx, float *res) { @@ -883,27 +894,17 @@ parse_section_main(struct context *ctx) return value_to_str(ctx, &conf->app_id); else if (strcmp(key, "initial-window-size-pixels") == 0) { - unsigned width, height; - if (sscanf(value, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) { - LOG_CONTEXTUAL_ERR("invalid size (must be on the form WIDTHxHEIGHT)"); + if (!value_to_dimensions(ctx, &conf->size.width, &conf->size.height)) return false; - } conf->size.type = CONF_SIZE_PX; - conf->size.width = width; - conf->size.height = height; } else if (strcmp(key, "initial-window-size-chars") == 0) { - unsigned width, height; - if (sscanf(value, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) { - LOG_CONTEXTUAL_ERR("invalid size (must be on the form WIDTHxHEIGHT)"); + if (!value_to_dimensions(ctx, &conf->size.width, &conf->size.height)) return false; - } conf->size.type = CONF_SIZE_CELLS; - conf->size.width = width; - conf->size.height = height; } else if (strcmp(key, "pad") == 0) { diff --git a/config.h b/config.h index 4b23e465..f0f95035 100644 --- a/config.h +++ b/config.h @@ -74,8 +74,8 @@ struct config { struct { enum conf_size_type type; - unsigned width; - unsigned height; + uint32_t width; + uint32_t height; } size; unsigned pad_x; From 205f1f7c02672b9ce7dafa4a9a77eff25de868b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 12:43:02 +0100 Subject: [PATCH 18/23] config: parse_section_main(): explicit return from each branch Add UNREACHABLE() at the bottom --- config.c | 52 +++++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/config.c b/config.c index fd646bb7..4e1b85c0 100644 --- a/config.c +++ b/config.c @@ -898,6 +898,7 @@ parse_section_main(struct context *ctx) return false; conf->size.type = CONF_SIZE_PX; + return true; } else if (strcmp(key, "initial-window-size-chars") == 0) { @@ -905,6 +906,7 @@ parse_section_main(struct context *ctx) return false; conf->size.type = CONF_SIZE_CELLS; + return true; } else if (strcmp(key, "pad") == 0) { @@ -924,6 +926,7 @@ parse_section_main(struct context *ctx) conf->pad_x = x; conf->pad_y = y; conf->center = center; + return true; } else if (strcmp(key, "resize-delay-ms") == 0) @@ -938,6 +941,7 @@ parse_section_main(struct context *ctx) return false; conf->bold_in_bright.palette_based = false; } + return true; } else if (strcmp(key, "bell") == 0) { @@ -968,6 +972,8 @@ parse_section_main(struct context *ctx) return false; } + + return true; } else if (strcmp(key, "initial-window-mode") == 0) { @@ -978,6 +984,7 @@ parse_section_main(struct context *ctx) return false; conf->startup_mode = mode; + return true; } else if (strcmp(key, "font") == 0 || @@ -997,32 +1004,26 @@ parse_section_main(struct context *ctx) config_font_list_destroy(&conf->fonts[idx]); conf->fonts[idx] = new_list; + return true; } - else if (strcmp(key, "line-height") == 0) { - if (!value_to_pt_or_px(ctx, &conf->line_height)) - return false; - } + else if (strcmp(key, "line-height") == 0) + return value_to_pt_or_px(ctx, &conf->line_height); - else if (strcmp(key, "letter-spacing") == 0) { - if (!value_to_pt_or_px(ctx, &conf->letter_spacing)) - return false; - } + else if (strcmp(key, "letter-spacing") == 0) + value_to_pt_or_px(ctx, &conf->letter_spacing); - else if (strcmp(key, "horizontal-letter-offset") == 0) { - if (!value_to_pt_or_px(ctx, &conf->horizontal_letter_offset)) - return false; - } + else if (strcmp(key, "horizontal-letter-offset") == 0) + return value_to_pt_or_px(ctx, &conf->horizontal_letter_offset); - else if (strcmp(key, "vertical-letter-offset") == 0) { - if (!value_to_pt_or_px(ctx, &conf->vertical_letter_offset)) - return false; - } + else if (strcmp(key, "vertical-letter-offset") == 0) + return value_to_pt_or_px(ctx, &conf->vertical_letter_offset); else if (strcmp(key, "underline-offset") == 0) { if (!value_to_pt_or_px(ctx, &conf->underline_offset)) return false; conf->use_custom_underline_offset = true; + return true; } else if (strcmp(key, "dpi-aware") == 0) { @@ -1034,6 +1035,7 @@ parse_section_main(struct context *ctx) return false; conf->dpi_aware = value ? DPI_AWARE_YES : DPI_AWARE_NO; } + return true; } else if (strcmp(key, "workers") == 0) @@ -1046,6 +1048,7 @@ parse_section_main(struct context *ctx) free(conf->word_delimiters); conf->word_delimiters = word_delimiters; + return true; } else if (strcmp(key, "jump-label-letters") == 0) { @@ -1058,23 +1061,20 @@ parse_section_main(struct context *ctx) free(conf->url.label_letters); conf->url.label_letters = letters; + return true; } - else if (strcmp(key, "notify") == 0) { - if (!value_to_spawn_template(ctx, &conf->notify)) - return false; - } + else if (strcmp(key, "notify") == 0) + return value_to_spawn_template(ctx, &conf->notify); - else if (strcmp(key, "notify-focus-inhibit") == 0) { + else if (strcmp(key, "notify-focus-inhibit") == 0) return value_to_bool(ctx, &conf->notify_focus_inhibit); - } else if (strcmp(key, "url-launch") == 0) { deprecated_url_option( conf, "url-launch", "launch", path, lineno); - if (!value_to_spawn_template(ctx, &conf->url.launch)) - return false; + return value_to_spawn_template(ctx, &conf->url.launch); } else if (strcmp(key, "selection-target") == 0) { @@ -1085,6 +1085,7 @@ parse_section_main(struct context *ctx) return false; conf->selection_target = target; + return true; } else if (strcmp(key, "osc8-underline") == 0) { @@ -1098,6 +1099,7 @@ parse_section_main(struct context *ctx) return false; conf->url.osc8_underline = mode; + return true; } else if (strcmp(key, "box-drawings-uses-font-glyphs") == 0) @@ -1108,7 +1110,7 @@ parse_section_main(struct context *ctx) return false; } - return true; + UNREACHABLE(); } static bool From 1f39c46c17fa3fab9384b28b304e838b2e30a086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 13:39:59 +0100 Subject: [PATCH 19/23] config: value_to_enum(), return conversion success/failure as a bool Result is set through a pointer passed as an argument. Note that we assume enums are 32-bit, and explicitly cast them to int*. Static asserts have been added to ensure this invariant holds. --- config.c | 172 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 92 insertions(+), 80 deletions(-) diff --git a/config.c b/config.c index 4e1b85c0..559f163d 100644 --- a/config.c +++ b/config.c @@ -592,15 +592,17 @@ value_to_wchars(struct context *ctx, wchar_t **res) return true; } -static int NOINLINE -value_to_enum(struct context *ctx, const char **value_map) +static bool NOINLINE +value_to_enum(struct context *ctx, const char **value_map, int *res) { size_t str_len = 0; size_t count = 0; for (; value_map[count] != NULL; count++) { - if (strcasecmp(value_map[count], ctx->value) == 0) - return count; + if (strcasecmp(value_map[count], ctx->value) == 0) { + *res = count; + return true; + } str_len += strlen(value_map[count]); } @@ -615,7 +617,8 @@ value_to_enum(struct context *ctx, const char **value_map) valid_values[idx - 2] = '\0'; LOG_CONTEXTUAL_ERR("not one of %s", valid_values); - return -1; + *res = -1; + return false; } static bool NOINLINE @@ -959,32 +962,32 @@ parse_section_main(struct context *ctx) }; tll_push_back(conf->notifications, deprecation); - int bell = value_to_enum( - ctx, (const char *[]){"set-urgency", "notify", "none", NULL}); - - switch (bell) { - case 0: conf->bell.urgent = true; break; - case 1: conf->bell.notify = true; break; - case 2: memset(&conf->bell, 0, sizeof(conf->bell)); break; - - case -1: - default: + int bell; + if (!value_to_enum( + ctx, + (const char *[]){"set-urgency", "notify", "none", NULL}, + &bell)) + { return false; - } - return true; + switch (bell) { + case 0: conf->bell.urgent = true; return true; + case 1: conf->bell.notify = true; return true; + case 2: memset(&conf->bell, 0, sizeof(conf->bell)); return true; + } + + UNREACHABLE(); } else if (strcmp(key, "initial-window-mode") == 0) { - int mode = value_to_enum( - ctx, (const char *[]){"windowed", "maximized", "fullscreen", NULL}); + _Static_assert(sizeof(conf->startup_mode) == sizeof(int), + "enum is not 32-bit"); - if (mode < 0) - return false; - - conf->startup_mode = mode; - return true; + return value_to_enum( + ctx, + (const char *[]){"windowed", "maximized", "fullscreen", NULL}, + (int *)&conf->startup_mode); } else if (strcmp(key, "font") == 0 || @@ -1078,28 +1081,26 @@ parse_section_main(struct context *ctx) } else if (strcmp(key, "selection-target") == 0) { - int target = value_to_enum( - ctx, (const char *[]){"none", "primary", "clipboard", "both", NULL}); + _Static_assert(sizeof(conf->selection_target) == sizeof(int), + "enum is not 32-bit"); - if (target < 0) - return false; - - conf->selection_target = target; - return true; + return value_to_enum( + ctx, + (const char *[]){"none", "primary", "clipboard", "both", NULL}, + (int *)&conf->selection_target); } else if (strcmp(key, "osc8-underline") == 0) { deprecated_url_option( conf, "osc8-underline", "osc8-underline", path, lineno); - int mode = value_to_enum( - ctx, (const char *[]){"url-mode", "always", NULL}); + _Static_assert(sizeof(conf->url.osc8_underline) == sizeof(int), + "enum is not 32-bit"); - if (mode < 0) - return false; - - conf->url.osc8_underline = mode; - return true; + return value_to_enum( + ctx, + (const char *[]){"url-mode", "always", NULL}, + (int *)&conf->url.osc8_underline); } else if (strcmp(key, "box-drawings-uses-font-glyphs") == 0) @@ -1147,13 +1148,14 @@ parse_section_scrollback(struct context *ctx) value_to_uint32(ctx, 10, &conf->scrollback.lines); else if (strcmp(key, "indicator-position") == 0) { - int position = value_to_enum( - ctx, (const char *[]){"none", "fixed", "relative", NULL}); + _Static_assert( + sizeof(conf->scrollback.indicator.position) == sizeof(int), + "enum is not 32-bit"); - if (position < 0) - return false; - - conf->scrollback.indicator.position = position; + return value_to_enum( + ctx, + (const char *[]){"none", "fixed", "relative", NULL}, + (int *)&conf->scrollback.indicator.position); } else if (strcmp(key, "indicator-format") == 0) { @@ -1211,13 +1213,13 @@ parse_section_url(struct context *ctx) } else if (strcmp(key, "osc8-underline") == 0) { - int mode = value_to_enum( - ctx, (const char *[]){"url-mode", "always", NULL}); + _Static_assert(sizeof(conf->url.osc8_underline) == sizeof(int), + "enum is not 32-bit"); - if (mode < 0) - return false; - - conf->url.osc8_underline = mode; + return value_to_enum( + ctx, + (const char *[]){"url-mode", "always", NULL}, + (int *)&conf->url.osc8_underline); } else if (strcmp(key, "protocols") == 0) { @@ -1397,13 +1399,13 @@ parse_section_cursor(struct context *ctx) const char *key = ctx->key; if (strcmp(key, "style") == 0) { - enum cursor_style style = value_to_enum( - ctx, (const char *[]){"block", "underline", "beam", NULL}); + _Static_assert(sizeof(conf->cursor.style) == sizeof(int), + "enum is not 32-bit"); - if (style < 0) - return false; - - conf->cursor.style = style; + return value_to_enum( + ctx, + (const char *[]){"block", "underline", "beam", NULL}, + (int *)&conf->cursor.style); } else if (strcmp(key, "blink") == 0) @@ -1468,13 +1470,13 @@ parse_section_csd(struct context *ctx) const char *key = ctx->key; if (strcmp(key, "preferred") == 0) { - int csd = value_to_enum( - ctx, (const char *[]){"none", "server", "client", NULL}); + _Static_assert(sizeof(conf->csd.preferred) == sizeof(int), + "enum is not 32-bit"); - if (csd < 0) - return false; - - conf->csd.preferred = csd; + return value_to_enum( + ctx, + (const char *[]){"none", "server", "client", NULL}, + (int *)&conf->csd.preferred); } else if (strcmp(key, "font") == 0) { @@ -2320,12 +2322,13 @@ parse_section_tweak(struct context *ctx) NULL, }; - int filter = value_to_enum(ctx, filters); - if (filter < 0) + _Static_assert(sizeof(conf->tweak.fcft_filter) == sizeof(int), + "enum is not 32-bit"); + + if (!value_to_enum(ctx, filters, (int *)&conf->tweak.fcft_filter)) return false; - conf->tweak.fcft_filter = filter; - LOG_WARN("tweak: scaling-filter=%s", filters[filter]); + LOG_WARN("tweak: scaling-filter=%s", filters[conf->tweak.fcft_filter]); } else if (strcmp(key, "overflowing-glyphs") == 0) { @@ -2367,45 +2370,54 @@ parse_section_tweak(struct context *ctx) } else if (strcmp(key, "grapheme-width-method") == 0) { - int method = value_to_enum( - ctx, (const char *[]){"wcswidth", "double-width", NULL}); + _Static_assert(sizeof(conf->tweak.grapheme_width_method) == sizeof(int), + "enum is not 32-bit"); - if (method < 0) + if (!value_to_enum( + ctx, + (const char *[]){"wcswidth", "double-width", NULL}, + (int *)&conf->tweak.grapheme_width_method)) + { return false; + } - conf->tweak.grapheme_width_method = method; LOG_WARN("%s:%d [tweak].grapheme-width-method=%s", path, lineno, value); } else if (strcmp(key, "render-timer") == 0) { - int mode = value_to_enum( - ctx, (const char *[]){"none", "osd", "log", "both", NULL}); + int mode; + + if (!value_to_enum( + ctx, + (const char *[]){"none", "osd", "log", "both", NULL}, + &mode)) + { + return false; + } switch (mode) { case 0: conf->tweak.render_timer_osd = false; conf->tweak.render_timer_log = false; - break; + return true; case 1: conf->tweak.render_timer_osd = true; conf->tweak.render_timer_log = false; - break; + return true; case 2: conf->tweak.render_timer_osd = false; conf->tweak.render_timer_log = true; - break; + return true; case 3: conf->tweak.render_timer_osd = true; conf->tweak.render_timer_log = true; - break; - - case -1: - default: - return false; + return true; } + + UNREACHABLE(); } else if (strcmp(key, "delayed-render-lower") == 0) { From d4b9ef7607fd1f410f9cadbb2456458e214e4834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 13:44:50 +0100 Subject: [PATCH 20/23] config: drop all warning logs from the tweak section --- config.c | 68 ++++++++++++---------------------------------- doc/foot.ini.5.scd | 4 +-- 2 files changed, 19 insertions(+), 53 deletions(-) diff --git a/config.c b/config.c index 559f163d..1c8165f8 100644 --- a/config.c +++ b/config.c @@ -2308,9 +2308,6 @@ parse_section_tweak(struct context *ctx) { struct config *conf = ctx->conf; const char *key = ctx->key; - const char *value = ctx->value; - const char *path = ctx->path; - unsigned lineno = ctx->lineno; if (strcmp(key, "scaling-filter") == 0) { static const char *filters[] = { @@ -2325,25 +2322,14 @@ parse_section_tweak(struct context *ctx) _Static_assert(sizeof(conf->tweak.fcft_filter) == sizeof(int), "enum is not 32-bit"); - if (!value_to_enum(ctx, filters, (int *)&conf->tweak.fcft_filter)) - return false; - - LOG_WARN("tweak: scaling-filter=%s", filters[conf->tweak.fcft_filter]); + return value_to_enum(ctx, filters, (int *)&conf->tweak.fcft_filter); } - else if (strcmp(key, "overflowing-glyphs") == 0) { - if (!value_to_bool(ctx, &conf->tweak.overflowing_glyphs)) - return false; - if (!conf->tweak.overflowing_glyphs) - LOG_WARN("tweak: disabled overflowing glyphs"); - } + else if (strcmp(key, "overflowing-glyphs") == 0) + return value_to_bool(ctx, &conf->tweak.overflowing_glyphs); - else if (strcmp(key, "damage-whole-window") == 0) { - if (!value_to_bool(ctx, &conf->tweak.damage_whole_window)) - return false; - if (conf->tweak.damage_whole_window) - LOG_WARN("tweak: damage whole window"); - } + else if (strcmp(key, "damage-whole-window") == 0) + return value_to_bool(ctx, &conf->tweak.damage_whole_window); else if (strcmp(key, "grapheme-shaping") == 0) { if (!value_to_bool(ctx, &conf->tweak.grapheme_shaping)) @@ -2365,23 +2351,17 @@ parse_section_tweak(struct context *ctx) * grapheme-clustering at least */ } - if (!conf->tweak.grapheme_shaping) - LOG_WARN("tweak: grapheme shaping disabled"); + return true; } else if (strcmp(key, "grapheme-width-method") == 0) { _Static_assert(sizeof(conf->tweak.grapheme_width_method) == sizeof(int), "enum is not 32-bit"); - if (!value_to_enum( - ctx, - (const char *[]){"wcswidth", "double-width", NULL}, - (int *)&conf->tweak.grapheme_width_method)) - { - return false; - } - - LOG_WARN("%s:%d [tweak].grapheme-width-method=%s", path, lineno, value); + return value_to_enum( + ctx, + (const char *[]){"wcswidth", "double-width", NULL}, + (int *)&conf->tweak.grapheme_width_method); } else if (strcmp(key, "render-timer") == 0) { @@ -2431,7 +2411,7 @@ parse_section_tweak(struct context *ctx) } conf->tweak.delayed_render_lower_ns = ns; - LOG_WARN("tweak: delayed-render-lower=%u", ns); + return true; } else if (strcmp(key, "delayed-render-upper") == 0) { @@ -2445,7 +2425,7 @@ parse_section_tweak(struct context *ctx) } conf->tweak.delayed_render_upper_ns = ns; - LOG_WARN("tweak: delayed-render-upper=%u", ns); + return true; } else if (strcmp(key, "max-shm-pool-size-mb") == 0) { @@ -2454,26 +2434,14 @@ parse_section_tweak(struct context *ctx) return false; conf->tweak.max_shm_pool_size = min((int32_t)mb * 1024 * 1024, INT32_MAX); - LOG_WARN("tweak: max-shm-pool-size=%lld bytes", - (long long)conf->tweak.max_shm_pool_size); + return true; } - else if (strcmp(key, "box-drawing-base-thickness") == 0) { - if (!value_to_double(ctx, &conf->tweak.box_drawing_base_thickness)) - return false; + else if (strcmp(key, "box-drawing-base-thickness") == 0) + return value_to_double(ctx, &conf->tweak.box_drawing_base_thickness); - LOG_WARN("tweak: box-drawing-base-thickness=%f", - conf->tweak.box_drawing_base_thickness); - } - - else if (strcmp(key, "box-drawing-solid-shades") == 0) { - if (!value_to_bool(ctx, &conf->tweak.box_drawing_solid_shades)) - return false; - - if (!conf->tweak.box_drawing_solid_shades) - LOG_WARN("tweak: box-drawing-solid-shades=%s", - conf->tweak.box_drawing_solid_shades ? "yes" : "no"); - } + else if (strcmp(key, "box-drawing-solid-shades") == 0) + return value_to_bool(ctx, &conf->tweak.box_drawing_solid_shades); else if (strcmp(key, "font-monospace-warn") == 0) return value_to_bool(ctx, &conf->tweak.font_monospace_warn); @@ -2483,7 +2451,7 @@ parse_section_tweak(struct context *ctx) return false; } - return true; + UNREACHABLE(); } static bool diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 456aa2a3..4445f0c7 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -894,9 +894,7 @@ This section is for advanced users and describes configuration options that can be used to tweak foot's low-level behavior. These options are *not* included in the example configuration. You -should not change these unless you understand what they do and note -that changing the default values *will* print a warning when launching -foot. +should not change these unless you understand what they do. Note that these options may change, or be removed at any time, without prior notice. From 2f0ab1da89de0a9db47b41863ddd822216707726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 14:22:26 +0100 Subject: [PATCH 21/23] config: appease compiler ../../foot/config.c:154:50: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security] log_msg(log_class, LOG_MODULE, file, lineno, formatted_msg); ^~~~~~~~~~~~~ ../../foot/config.c:154:50: note: treat the string as an argument to avoid this log_msg(log_class, LOG_MODULE, file, lineno, formatted_msg) --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index 1c8165f8..9f77ef22 100644 --- a/config.c +++ b/config.c @@ -151,7 +151,7 @@ log_and_notify_va(struct config *conf, enum log_class log_class, } char *formatted_msg = xvasprintf(fmt, va); - log_msg(log_class, LOG_MODULE, file, lineno, formatted_msg); + log_msg(log_class, LOG_MODULE, file, lineno, "%s", formatted_msg); tll_push_back( conf->notifications, ((struct user_notification){.kind = kind, .text = formatted_msg})); From 73c5754f5d4c7d6588a4acf635723e69eac7bf76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 6 Nov 2021 20:32:35 +0100 Subject: [PATCH 22/23] config: value_to_wchars(): free the target variable --- config.c | 45 +++++++++++---------------------------------- 1 file changed, 11 insertions(+), 34 deletions(-) diff --git a/config.c b/config.c index 9f77ef22..09b4285f 100644 --- a/config.c +++ b/config.c @@ -579,14 +579,13 @@ value_to_str(struct context *ctx, char **res) static bool NOINLINE value_to_wchars(struct context *ctx, wchar_t **res) { - *res = NULL; - size_t chars = mbstowcs(NULL, ctx->value, 0); if (chars == (size_t)-1) { LOG_CONTEXTUAL_ERR("not a valid string value"); return false; } + free(*res); *res = xmalloc((chars + 1) * sizeof(wchar_t)); mbstowcs(*res, ctx->value, chars + 1); return true; @@ -1044,27 +1043,14 @@ parse_section_main(struct context *ctx) else if (strcmp(key, "workers") == 0) return value_to_uint16(ctx, 10, &conf->render_worker_count); - else if (strcmp(key, "word-delimiters") == 0) { - wchar_t *word_delimiters; - if (!value_to_wchars(ctx, &word_delimiters)) - return false; - - free(conf->word_delimiters); - conf->word_delimiters = word_delimiters; - return true; - } + else if (strcmp(key, "word-delimiters") == 0) + return value_to_wchars(ctx, &conf->word_delimiters); else if (strcmp(key, "jump-label-letters") == 0) { deprecated_url_option( conf, "jump-label-letters", "label-letters", path, lineno); - wchar_t *letters; - if (!value_to_wchars(ctx, &letters)) - return false; - - free(conf->url.label_letters); - conf->url.label_letters = letters; - return true; + return value_to_wchars(ctx, &conf->url.label_letters); } else if (strcmp(key, "notify") == 0) @@ -1203,14 +1189,8 @@ parse_section_url(struct context *ctx) return false; } - else if (strcmp(key, "label-letters") == 0) { - wchar_t *letters; - if (!value_to_wchars(ctx, &letters)) - return false; - - free(conf->url.label_letters); - conf->url.label_letters = letters; - } + else if (strcmp(key, "label-letters") == 0) + return value_to_wchars(ctx, &conf->url.label_letters); else if (strcmp(key, "osc8-underline") == 0) { _Static_assert(sizeof(conf->url.osc8_underline) == sizeof(int), @@ -1273,18 +1253,15 @@ parse_section_url(struct context *ctx) } else if (strcmp(key, "uri-characters") == 0) { - wchar_t *uri_characters; - if (!value_to_wchars(ctx, &uri_characters)) + if (!value_to_wchars(ctx, &conf->url.uri_characters)) return false; - free(conf->url.uri_characters); - qsort( - uri_characters, - wcslen(uri_characters), - sizeof(uri_characters[0]), + conf->url.uri_characters, + wcslen(conf->url.uri_characters), + sizeof(conf->url.uri_characters[0]), &wccmp); - conf->url.uri_characters = uri_characters; + return true; } else { From b5f7c414a96099ceceed6355f3e45895ecf85f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 7 Nov 2021 11:35:39 +0100 Subject: [PATCH 23/23] changelog: boolean options --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98d85440..d6b3b1a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,10 @@ * Foot now terminates if there are no available seats - for example, due to the compositor not implementing a recent enough version of the `wl_seat` interface (https://codeberg.org/dnkl/foot/issues/779). +* Boolean options in `foot.ini` are now limited to + “yes|true|on|1|no|false|off|0”, Previously, anything that did not + match “yes|true|on”, or a number greater than 0, was treated as + “false”. ### Deprecated