From 97f9f0b699316ba60009b395948a712ec0b671d2 Mon Sep 17 00:00:00 2001 From: Brian Ashworth Date: Fri, 27 Dec 2019 23:33:55 -0500 Subject: parse_color: return success + drop fallback color This is the first in a series of commits to refactor the color handling in sway. This changes parse_color to return whether it was success and no longer uses 0xFFFFFFFF as the fallback color. This also verifies that the string actually contains a valid hexadecimal number along with the length checks. In the process of altering the calls to parse_color, I also took the opportunity to heavily refactor swaybar's ipc_parse_colors function. This allowed for several lines of duplicated code to be removed. --- common/util.c | 19 ++++--- include/swaybar/i3bar.h | 3 +- include/util.h | 7 ++- swaybar/i3bar.c | 20 ++++--- swaybar/ipc.c | 147 ++++++++++++------------------------------------ swaybar/render.c | 2 +- swaynag/config.c | 20 +++---- 7 files changed, 76 insertions(+), 142 deletions(-) diff --git a/common/util.c b/common/util.c index c0324b2f..84ebab99 100644 --- a/common/util.c +++ b/common/util.c @@ -1,4 +1,5 @@ #define _POSIX_C_SOURCE 200809L +#include #include #include #include @@ -13,21 +14,21 @@ int wrap(int i, int max) { return ((i % max) + max) % max; } -uint32_t parse_color(const char *color) { +bool parse_color(const char *color, uint32_t *result) { if (color[0] == '#') { ++color; } - int len = strlen(color); - if (len != 6 && len != 8) { - sway_log(SWAY_DEBUG, "Invalid color %s, defaulting to color 0xFFFFFFFF", color); - return 0xFFFFFFFF; + if ((len != 6 && len != 8) || !isxdigit(color[0]) || !isxdigit(color[1])) { + return false; } - uint32_t res = (uint32_t)strtoul(color, NULL, 16); - if (strlen(color) == 6) { - res = (res << 8) | 0xFF; + char *ptr; + uint32_t parsed = strtoul(color, &ptr, 16); + if (*ptr != '\0') { + return false; } - return res; + *result = len == 6 ? ((parsed << 8) | 0xFF) : parsed; + return true; } bool parse_boolean(const char *boolean, bool current) { diff --git a/include/swaybar/i3bar.h b/include/swaybar/i3bar.h index 5b6001ce..0b3bee21 100644 --- a/include/swaybar/i3bar.h +++ b/include/swaybar/i3bar.h @@ -9,7 +9,8 @@ struct i3bar_block { int ref_count; char *full_text, *short_text, *align, *min_width_str; bool urgent; - uint32_t *color; + uint32_t color; + bool color_set; int min_width; char *name, *instance; bool separator; diff --git a/include/util.h b/include/util.h index 3cba49f0..931ac691 100644 --- a/include/util.h +++ b/include/util.h @@ -11,10 +11,11 @@ int wrap(int i, int max); /** - * Given a string that represents an RGB(A) color, return a uint32_t - * version of the color. + * Given a string that represents an RGB(A) color, result will be set to a + * uint32_t version of the color, as long as it is valid. If it is invalid, + * then false will be returned and result will be untouched. */ -uint32_t parse_color(const char *color); +bool parse_color(const char *color, uint32_t *result); /** * Given a string that represents a boolean, return the boolean value. This diff --git a/swaybar/i3bar.c b/swaybar/i3bar.c index 43e2fe2d..5c8b87a2 100644 --- a/swaybar/i3bar.c +++ b/swaybar/i3bar.c @@ -24,7 +24,6 @@ void i3bar_block_unref(struct i3bar_block *block) { free(block->min_width_str); free(block->name); free(block->instance); - free(block->color); free(block); } } @@ -70,8 +69,11 @@ static void i3bar_parse_json(struct status_line *status, block->short_text = short_text ? strdup(json_object_get_string(short_text)) : NULL; if (color) { - block->color = malloc(sizeof(uint32_t)); - *block->color = parse_color(json_object_get_string(color)); + const char *hexstring = json_object_get_string(color); + block->color_set = parse_color(hexstring, &block->color); + if (!block->color_set) { + sway_log(SWAY_ERROR, "Invalid block color: %s", hexstring); + } } if (min_width) { json_type type = json_object_get_type(min_width); @@ -98,10 +100,14 @@ static void i3bar_parse_json(struct status_line *status, block->separator_block_width = separator_block_width ? json_object_get_int(separator_block_width) : 9; // Airblader features - block->background = background ? - parse_color(json_object_get_string(background)) : 0; - block->border = border ? - parse_color(json_object_get_string(border)) : 0; + const char *hex = background ? json_object_get_string(background) : NULL; + if (hex && !parse_color(hex, &block->background)) { + sway_log(SWAY_ERROR, "Ignoring invalid block background: %s", hex); + } + hex = border ? json_object_get_string(border) : NULL; + if (hex && !parse_color(hex, &block->border)) { + sway_log(SWAY_ERROR, "Ignoring invalid block border: %s", hex); + } block->border_top = border_top ? json_object_get_int(border_top) : 1; block->border_bottom = border_bottom ? json_object_get_int(border_bottom) : 1; diff --git a/swaybar/ipc.c b/swaybar/ipc.c index afaffb04..cca510c6 100644 --- a/swaybar/ipc.c +++ b/swaybar/ipc.c @@ -55,117 +55,42 @@ char *parse_font(const char *font) { static void ipc_parse_colors( struct swaybar_config *config, json_object *colors) { - json_object *background, *statusline, *separator; - json_object *focused_background, *focused_statusline, *focused_separator; - json_object *focused_workspace_border, *focused_workspace_bg, *focused_workspace_text; - json_object *inactive_workspace_border, *inactive_workspace_bg, *inactive_workspace_text; - json_object *active_workspace_border, *active_workspace_bg, *active_workspace_text; - json_object *urgent_workspace_border, *urgent_workspace_bg, *urgent_workspace_text; - json_object *binding_mode_border, *binding_mode_bg, *binding_mode_text; - json_object_object_get_ex(colors, "background", &background); - json_object_object_get_ex(colors, "statusline", &statusline); - json_object_object_get_ex(colors, "separator", &separator); - json_object_object_get_ex(colors, "focused_background", &focused_background); - json_object_object_get_ex(colors, "focused_statusline", &focused_statusline); - json_object_object_get_ex(colors, "focused_separator", &focused_separator); - json_object_object_get_ex(colors, "focused_workspace_border", &focused_workspace_border); - json_object_object_get_ex(colors, "focused_workspace_bg", &focused_workspace_bg); - json_object_object_get_ex(colors, "focused_workspace_text", &focused_workspace_text); - json_object_object_get_ex(colors, "active_workspace_border", &active_workspace_border); - json_object_object_get_ex(colors, "active_workspace_bg", &active_workspace_bg); - json_object_object_get_ex(colors, "active_workspace_text", &active_workspace_text); - json_object_object_get_ex(colors, "inactive_workspace_border", &inactive_workspace_border); - json_object_object_get_ex(colors, "inactive_workspace_bg", &inactive_workspace_bg); - json_object_object_get_ex(colors, "inactive_workspace_text", &inactive_workspace_text); - json_object_object_get_ex(colors, "urgent_workspace_border", &urgent_workspace_border); - json_object_object_get_ex(colors, "urgent_workspace_bg", &urgent_workspace_bg); - json_object_object_get_ex(colors, "urgent_workspace_text", &urgent_workspace_text); - json_object_object_get_ex(colors, "binding_mode_border", &binding_mode_border); - json_object_object_get_ex(colors, "binding_mode_bg", &binding_mode_bg); - json_object_object_get_ex(colors, "binding_mode_text", &binding_mode_text); - if (background) { - config->colors.background = parse_color( - json_object_get_string(background)); - } - if (statusline) { - config->colors.statusline = parse_color( - json_object_get_string(statusline)); - } - if (separator) { - config->colors.separator = parse_color( - json_object_get_string(separator)); - } - if (focused_background) { - config->colors.focused_background = parse_color( - json_object_get_string(focused_background)); - } - if (focused_statusline) { - config->colors.focused_statusline = parse_color( - json_object_get_string(focused_statusline)); - } - if (focused_separator) { - config->colors.focused_separator = parse_color( - json_object_get_string(focused_separator)); - } - if (focused_workspace_border) { - config->colors.focused_workspace.border = parse_color( - json_object_get_string(focused_workspace_border)); - } - if (focused_workspace_bg) { - config->colors.focused_workspace.background = parse_color( - json_object_get_string(focused_workspace_bg)); - } - if (focused_workspace_text) { - config->colors.focused_workspace.text = parse_color( - json_object_get_string(focused_workspace_text)); - } - if (active_workspace_border) { - config->colors.active_workspace.border = parse_color( - json_object_get_string(active_workspace_border)); - } - if (active_workspace_bg) { - config->colors.active_workspace.background = parse_color( - json_object_get_string(active_workspace_bg)); - } - if (active_workspace_text) { - config->colors.active_workspace.text = parse_color( - json_object_get_string(active_workspace_text)); - } - if (inactive_workspace_border) { - config->colors.inactive_workspace.border = parse_color( - json_object_get_string(inactive_workspace_border)); - } - if (inactive_workspace_bg) { - config->colors.inactive_workspace.background = parse_color( - json_object_get_string(inactive_workspace_bg)); - } - if (inactive_workspace_text) { - config->colors.inactive_workspace.text = parse_color( - json_object_get_string(inactive_workspace_text)); - } - if (urgent_workspace_border) { - config->colors.urgent_workspace.border = parse_color( - json_object_get_string(urgent_workspace_border)); - } - if (urgent_workspace_bg) { - config->colors.urgent_workspace.background = parse_color( - json_object_get_string(urgent_workspace_bg)); - } - if (urgent_workspace_text) { - config->colors.urgent_workspace.text = parse_color( - json_object_get_string(urgent_workspace_text)); - } - if (binding_mode_border) { - config->colors.binding_mode.border = parse_color( - json_object_get_string(binding_mode_border)); - } - if (binding_mode_bg) { - config->colors.binding_mode.background = parse_color( - json_object_get_string(binding_mode_bg)); - } - if (binding_mode_text) { - config->colors.binding_mode.text = parse_color( - json_object_get_string(binding_mode_text)); + struct { + const char *name; + uint32_t *color; + } properties[] = { + { "background", &config->colors.background }, + { "statusline", &config->colors.statusline }, + { "separator", &config->colors.separator }, + { "focused_background", &config->colors.focused_background }, + { "focused_statusline", &config->colors.focused_statusline }, + { "focused_separator", &config->colors.focused_separator }, + { "focused_workspace_border", &config->colors.focused_workspace.border }, + { "focused_workspace_bg", &config->colors.focused_workspace.background }, + { "focused_workspace_text", &config->colors.focused_workspace.text }, + { "active_workspace_border", &config->colors.active_workspace.border }, + { "active_workspace_bg", &config->colors.active_workspace.background }, + { "active_workspace_text", &config->colors.active_workspace.text }, + { "inactive_workspace_border", &config->colors.inactive_workspace.border }, + { "inactive_workspace_bg", &config->colors.inactive_workspace.background }, + { "inactive_workspace_text", &config->colors.inactive_workspace.text }, + { "urgent_workspace_border", &config->colors.urgent_workspace.border }, + { "urgent_workspace_bg", &config->colors.urgent_workspace.background }, + { "urgent_workspace_text", &config->colors.urgent_workspace.text }, + { "binding_mode_border", &config->colors.binding_mode.border }, + { "binding_mode_bg", &config->colors.binding_mode.background }, + { "binding_mode_text", &config->colors.binding_mode.text }, + }; + + for (size_t i = 0; i < sizeof(properties) / sizeof(properties[i]); i++) { + json_object *object; + if (json_object_object_get_ex(colors, properties[i].name, &object)) { + const char *hexstring = json_object_get_string(object); + if (!parse_color(hexstring, properties[i].color)) { + sway_log(SWAY_ERROR, "Ignoring invalid %s: %s", + properties[i].name, hexstring); + } + } } } diff --git a/swaybar/render.c b/swaybar/render.c index 0d6bb354..06efb53c 100644 --- a/swaybar/render.c +++ b/swaybar/render.c @@ -265,7 +265,7 @@ static uint32_t render_status_block(cairo_t *cairo, } double text_y = height / 2.0 - text_height / 2.0; cairo_move_to(cairo, offset, (int)floor(text_y)); - uint32_t color = block->color ? *block->color : config->colors.statusline; + uint32_t color = block->color_set ? block->color : config->colors.statusline; color = block->urgent ? config->colors.urgent_workspace.text : color; cairo_set_source_u32(cairo, color); pango_printf(cairo, config->font, output->scale, diff --git a/swaynag/config.c b/swaynag/config.c index 2fa7cb61..f1161b39 100644 --- a/swaynag/config.c +++ b/swaynag/config.c @@ -221,28 +221,28 @@ int swaynag_parse_options(int argc, char **argv, struct swaynag *swaynag, fprintf(stdout, "swaynag version " SWAY_VERSION "\n"); return -1; case TO_COLOR_BACKGROUND: // Background color - if (type) { - type->background = parse_color(optarg); + if (type && !parse_color(optarg, &type->background)) { + fprintf(stderr, "Invalid background color: %s", optarg); } break; case TO_COLOR_BORDER: // Border color - if (type) { - type->border = parse_color(optarg); + if (type && !parse_color(optarg, &type->border)) { + fprintf(stderr, "Invalid border color: %s", optarg); } break; case TO_COLOR_BORDER_BOTTOM: // Bottom border color - if (type) { - type->border_bottom = parse_color(optarg); + if (type && !parse_color(optarg, &type->border_bottom)) { + fprintf(stderr, "Invalid border bottom color: %s", optarg); } break; case TO_COLOR_BUTTON: // Button background color - if (type) { - type->button_background = parse_color(optarg); + if (type && !parse_color(optarg, &type->button_background)) { + fprintf(stderr, "Invalid button background color: %s", optarg); } break; case TO_COLOR_TEXT: // Text color - if (type) { - type->text = parse_color(optarg); + if (type && !parse_color(optarg, &type->text)) { + fprintf(stderr, "Invalid text color: %s", optarg); } break; case TO_THICK_BAR_BORDER: // Bottom border thickness -- cgit v1.2.3-54-g00ecf