[PATCH weston 1/2] config-parser: Add weston_config_section_get_color
Bryce Harrington
bryce at osg.samsung.com
Thu Jul 14 18:31:33 UTC 2016
On Thu, Jul 14, 2016 at 09:40:46AM -0700, Yong Bakos wrote:
> 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
Ok sure, can add that.
Bryce
> 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