[PATCH weston 1/2] config-parser: Add weston_config_section_get_color
Yong Bakos
junk at humanoriented.com
Thu Jul 14 16:40:46 UTC 2016
On Jul 13, 2016, at 7:01 PM, Bryce Harrington <bryce at osg.samsung.com> wrote:
>
> 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", "01234567", and "1234567" all result in
> the same color value, "99999999" is grey, and "ffffffff" is white.
>
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
I support the intent here, since it helps separate the *get_uint calls
from the ones for parsing hex colors, leaving open the opportunity to
further refactor.
But, there seems to be something missing from the tests, to assert that
> "0x01234567", "01234567", and "1234567" all result in
> the same color value
yong
> ---
> clients/desktop-shell.c | 8 ++--
> clients/ivi-shell-user-interface.c | 2 +-
> shared/config-parser.c | 27 +++++++++++++
> shared/config-parser.h | 4 ++
> tests/config-parser-test.c | 80 ++++++++++++++++++++++++++++++++++++++
> 5 files changed, 116 insertions(+), 5 deletions(-)
>
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index f5e0ba2..18b8f9e 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 06b79a2..465def1 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 1e08759..f040380 100644
> --- a/shared/config-parser.c
> +++ b/shared/config-parser.c
> @@ -209,6 +209,33 @@ weston_config_section_get_uint(struct weston_config_section *section,
>
> 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;
> + char *end;
> +
> + entry = config_section_get_entry(section, key);
> + if (entry == NULL) {
> + *color = default_color;
> + errno = ENOENT;
> + 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,
> const char *key,
> double *value, double default_value)
> diff --git a/shared/config-parser.h b/shared/config-parser.h
> index b8462a7..17ef5d7 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 735da4e..f9495b3 100644
> --- a/tests/config-parser-test.c
> +++ b/tests/config-parser-test.c
> @@ -118,6 +118,9 @@ static struct zuc_fixture config_test_t1 = {
> "number=5252\n"
> "zero=0\n"
> "flag=false\n"
> + "color-none=0x000000\n"
> + "color-low=0x11223344\n"
> + "color-high=0xff00ff00\n"
> "\n"
> "[stuff]\n"
> "flag= true \n"
> @@ -461,6 +464,83 @@ 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, "bar", NULL, NULL);
> + r = weston_config_section_get_color(section, "color-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, "bar", NULL, NULL);
> + r = weston_config_section_get_color(section, "color-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, "bar", NULL, NULL);
> + r = weston_config_section_get_color(section, "color-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, "bar", NULL, NULL);
> + r = weston_config_section_get_color(section, "number", &n, 0xff336699);
> +
> + ZUC_ASSERT_EQ(0, r);
> + ZUC_ASSERT_EQ(0x5252, 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;
> +
> + // 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;
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list