From e776f2a4d9a335a41d9cbc8b06ba97a660051614 Mon Sep 17 00:00:00 2001 From: Bryce Harrington Date: Thu, 14 Jul 2016 18:28:03 -0700 Subject: [PATCH] config-parser: Add weston_config_section_get_color Previously weston_config_section_get_uint was serving dual purpose for parsing both unsigned decimal integer values (ids, counts, seconds, etc.) and hexadecimal values (colors), by relying on strtoul's auto-detection mechanism. However, this usage is unable to catch certain kinds of error conditions, such as specifying a negative number where an unsigned should be used. And for colors in particular, it would misparse hex values if the leading 0x was omitted. E.g. "background-color=99999999" would render a near-black background (effectively 0x05f5e0ff) instead of medium grey, and "background-color=ffffffff" would be treated as an error rather than white. "background-color=0x01234567", "background-color=01234567", and "background-color=1234567" each resulted in the value being parsed as hexadecimal, octal, and decimal respectively, resulting in colors 0x01234567, 0x00053977, and 0x0012d687 being displayed. This new routine forces hexadecimal to be used in all cases when parsing color values, so "0x01234567" and "01234567" result in the same color value, "99999999" is grey, and "ffffffff" is white. It also requires exactly 8 or 10 digits (other lengths likely indicate typos), or the value "0" (black). Signed-off-by: Bryce Harrington Reviewed-by: Eric Engestrom --- clients/desktop-shell.c | 8 +- clients/ivi-shell-user-interface.c | 2 +- shared/config-parser.c | 39 ++++++++++ shared/config-parser.h | 4 + tests/config-parser-test.c | 121 ++++++++++++++++++++++++++++- 5 files changed, 167 insertions(+), 7 deletions(-) diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index f5e0ba2e..18b8f9ea 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -571,8 +571,8 @@ panel_create(struct desktop *desktop) free (clock_format_option); s = weston_config_get_section(desktop->config, "shell", NULL, NULL); - weston_config_section_get_uint(s, "panel-color", - &panel->color, 0xaa000000); + weston_config_section_get_color(s, "panel-color", + &panel->color, 0xaa000000); panel_add_launchers(panel, desktop); @@ -1068,8 +1068,8 @@ background_create(struct desktop *desktop) s = weston_config_get_section(desktop->config, "shell", NULL, NULL); weston_config_section_get_string(s, "background-image", &background->image, NULL); - weston_config_section_get_uint(s, "background-color", - &background->color, 0); + weston_config_section_get_color(s, "background-color", + &background->color, 0x00000000); weston_config_section_get_string(s, "background-type", &type, "tile"); diff --git a/clients/ivi-shell-user-interface.c b/clients/ivi-shell-user-interface.c index 06b79a22..465def1a 100644 --- a/clients/ivi-shell-user-interface.c +++ b/clients/ivi-shell-user-interface.c @@ -1143,7 +1143,7 @@ hmi_homescreen_setting_create(void) weston_config_section_get_uint( shellSection, "home-id", &setting->home.id, 1007); - weston_config_section_get_uint( + weston_config_section_get_color( shellSection, "workspace-background-color", &setting->workspace_background.color, 0x99000000); diff --git a/shared/config-parser.c b/shared/config-parser.c index 1e08759e..33d5e361 100644 --- a/shared/config-parser.c +++ b/shared/config-parser.c @@ -207,6 +207,45 @@ weston_config_section_get_uint(struct weston_config_section *section, return 0; } +WL_EXPORT +int +weston_config_section_get_color(struct weston_config_section *section, + const char *key, + uint32_t *color, uint32_t default_color) +{ + struct weston_config_entry *entry; + int len; + char *end; + + entry = config_section_get_entry(section, key); + if (entry == NULL) { + *color = default_color; + errno = ENOENT; + return -1; + } + + len = strlen(entry->value); + if (len == 1 && entry->value[0] == '0') { + *color = 0; + return 0; + } else if (len != 8 && len != 10) { + fprintf(stderr, "string '%s' is length %d\n", entry->value, len); + *color = default_color; + errno = EINVAL; + return -1; + } + + errno = 0; + *color = strtoul(entry->value, &end, 16); + if (errno != 0 || end == entry->value || *end != '\0') { + *color = default_color; + errno = EINVAL; + return -1; + } + + return 0; +} + WL_EXPORT int weston_config_section_get_double(struct weston_config_section *section, diff --git a/shared/config-parser.h b/shared/config-parser.h index b8462a72..17ef5d73 100644 --- a/shared/config-parser.h +++ b/shared/config-parser.h @@ -85,6 +85,10 @@ weston_config_section_get_uint(struct weston_config_section *section, const char *key, uint32_t *value, uint32_t default_value); int +weston_config_section_get_color(struct weston_config_section *section, + const char *key, + uint32_t *color, uint32_t default_color); +int weston_config_section_get_double(struct weston_config_section *section, const char *key, double *value, double default_value); diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c index 735da4e0..5a5a380b 100644 --- a/tests/config-parser-test.c +++ b/tests/config-parser-test.c @@ -119,6 +119,14 @@ static struct zuc_fixture config_test_t1 = { "zero=0\n" "flag=false\n" "\n" + "[colors]\n" + "none=0x00000000\n" + "low=0x11223344\n" + "high=0xff00ff00\n" + "oct=01234567\n" + "dec=12345670\n" + "short=1234567\n" + "\n" "[stuff]\n" "flag= true \n" "\n" @@ -136,7 +144,7 @@ static struct zuc_fixture config_test_t1 = { }; static const char *section_names[] = { - "foo", "bar", "stuff", "bucket", "bucket" + "foo", "bar", "colors", "stuff", "bucket", "bucket" }; /* @@ -428,7 +436,7 @@ ZUC_TEST_F(config_test_t1, test017, data) while (weston_config_next_section(config, §ion, &name)) ZUC_ASSERT_STREQ(section_names[i++], name); - ZUC_ASSERT_EQ(5, i); + ZUC_ASSERT_EQ(6, i); } ZUC_TEST_F(config_test_t1, test018, data) @@ -461,6 +469,115 @@ ZUC_TEST_F(config_test_t1, test019, data) ZUC_ASSERT_EQ(0, errno); } +ZUC_TEST_F(config_test_t1, test020, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + section = weston_config_get_section(config, "colors", NULL, NULL); + r = weston_config_section_get_color(section, "none", &n, 0xff336699); + + ZUC_ASSERT_EQ(0, r); + ZUC_ASSERT_EQ(0x000000, n); + ZUC_ASSERT_EQ(0, errno); +} + +ZUC_TEST_F(config_test_t1, test021, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + section = weston_config_get_section(config, "colors", NULL, NULL); + r = weston_config_section_get_color(section, "low", &n, 0xff336699); + + ZUC_ASSERT_EQ(0, r); + ZUC_ASSERT_EQ(0x11223344, n); + ZUC_ASSERT_EQ(0, errno); +} + +ZUC_TEST_F(config_test_t1, test022, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + section = weston_config_get_section(config, "colors", NULL, NULL); + r = weston_config_section_get_color(section, "high", &n, 0xff336699); + + ZUC_ASSERT_EQ(0, r); + ZUC_ASSERT_EQ(0xff00ff00, n); + ZUC_ASSERT_EQ(0, errno); +} + +ZUC_TEST_F(config_test_t1, test023, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + // Treat colors as hex values even if missing the leading 0x + section = weston_config_get_section(config, "colors", NULL, NULL); + r = weston_config_section_get_color(section, "oct", &n, 0xff336699); + + ZUC_ASSERT_EQ(0, r); + ZUC_ASSERT_EQ(0x01234567, n); + ZUC_ASSERT_EQ(0, errno); +} + +ZUC_TEST_F(config_test_t1, test024, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + // Treat colors as hex values even if missing the leading 0x + section = weston_config_get_section(config, "colors", NULL, NULL); + r = weston_config_section_get_color(section, "dec", &n, 0xff336699); + + ZUC_ASSERT_EQ(0, r); + ZUC_ASSERT_EQ(0x12345670, n); + ZUC_ASSERT_EQ(0, errno); +} + +ZUC_TEST_F(config_test_t1, test025, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + // 7-digit colors are not valid (most likely typos) + section = weston_config_get_section(config, "colors", NULL, NULL); + r = weston_config_section_get_color(section, "short", &n, 0xff336699); + + ZUC_ASSERT_EQ(-1, r); + ZUC_ASSERT_EQ(0xff336699, n); + ZUC_ASSERT_EQ(EINVAL, errno); +} + +ZUC_TEST_F(config_test_t1, test026, data) +{ + int r; + uint32_t n; + struct weston_config_section *section; + struct weston_config *config = data; + + // String color names are unsupported + section = weston_config_get_section(config, "bucket", NULL, NULL); + r = weston_config_section_get_color(section, "color", &n, 0xff336699); + + ZUC_ASSERT_EQ(-1, r); + ZUC_ASSERT_EQ(0xff336699, n); + ZUC_ASSERT_EQ(EINVAL, errno); +} + ZUC_TEST_F(config_test_t2, doesnt_parse, data) { struct weston_config *config = data;