[PATCH weston 1/2] config-parser: Add weston_config_section_get_color

Quentin Glidic sardemff7+wayland at sardemff7.net
Thu Jul 14 07:16:41 UTC 2016


On 14/07/2016 04:01, Bryce Harrington 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>
> ---

If we add a specific helper, I would use more specific parsing, i.e. 
fully support CSS-compatible # notation.

It is not that hard: check if the first char is #, then split by groups 
of 2 chars to be parsed as hex. Alpha is then the last part #RRGGBBAA 
and we can also support the one-char-per-colour version #RGB(A).

I can make a patch quickly if you want, with backward compatibility (if 
prefix is 0x, use 0x(AA)RRGGBB).

Cheers,


>  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;
>


-- 

Quentin “Sardem FF7” Glidic


More information about the wayland-devel mailing list