[PATCH weston] config-parser: Improve error checks for strtol/strtoul calls

[Eric Engestrom]

On Thu, Jul 07, 2016 at 02:08:28PM -0700, Bryce Harrington wrote:
> Check errno, which is set of over/underflow, out of range, etc.  Also
> check for empty strings (the usages covered in this patch already also
> cover the case where there are non-digits present).  Set errno to 0
> before making the strto*l call in case of pre-existing errors
> (i.e. ENOTTY when running under the testsuite).
> 
> This follows the error checking style used in Wayland
> (c.f. wayland-client.c and scanner.c).
> 
> In tests, also check errno, and add testcases for parsing '0'.
> 
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  shared/config-parser.c     |  6 ++++--
>  tests/config-parser-test.c | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/shared/config-parser.c b/shared/config-parser.c
> index 2256469..151ae9c 100644
> --- a/shared/config-parser.c
> +++ b/shared/config-parser.c
> @@ -169,8 +169,9 @@ weston_config_section_get_int(struct weston_config_section *section,
>  		return -1;
>  	}
>  
> +	errno = 0;
>  	*value = strtol(entry->value, &end, 0);
> -	if (*end != '\0') {
> +	if (errno != 0 || end == entry->value || *end != '\0') {

Isn't the empty string case already covered by `*end != '\0'` ?
Either way, the duplicate test wouldn't hurt, so:
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

>  		*value = default_value;
>  		errno = EINVAL;
>  		return -1;
> @@ -195,8 +196,9 @@ weston_config_section_get_uint(struct weston_config_section *section,
>  		return -1;
>  	}
>  
> +	errno = 0;
>  	*value = strtoul(entry->value, &end, 0);
> -	if (*end != '\0') {
> +	if (errno != 0 || end == entry->value || *end != '\0') {
>  		*value = default_value;
>  		errno = EINVAL;
>  		return -1;
> diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c
> index 5dcafc4..735da4e 100644
> --- a/tests/config-parser-test.c
> +++ b/tests/config-parser-test.c
> @@ -116,6 +116,7 @@ static struct zuc_fixture config_test_t1 = {
>  	"[bar]\n"
>  	"# more comments\n"
>  	"number=5252\n"
> +	"zero=0\n"
>  	"flag=false\n"
>  	"\n"
>  	"[stuff]\n"
> @@ -263,6 +264,7 @@ ZUC_TEST_F(config_test_t1, test006, data)
>  
>  	ZUC_ASSERT_EQ(0, r);
>  	ZUC_ASSERT_EQ(5252, n);
> +	ZUC_ASSERT_EQ(0, errno);
>  }
>  
>  ZUC_TEST_F(config_test_t1, test007, data)
> @@ -289,8 +291,10 @@ ZUC_TEST_F(config_test_t1, test008, data)
>  
>  	section = weston_config_get_section(config, "bar", NULL, NULL);
>  	r = weston_config_section_get_uint(section, "number", &u, 600);
> +
>  	ZUC_ASSERT_EQ(0, r);
>  	ZUC_ASSERT_EQ(5252, u);
> +	ZUC_ASSERT_EQ(0, errno);
>  }
>  
>  ZUC_TEST_F(config_test_t1, test009, data)
> @@ -427,6 +431,36 @@ ZUC_TEST_F(config_test_t1, test017, data)
>  	ZUC_ASSERT_EQ(5, i);
>  }
>  
> +ZUC_TEST_F(config_test_t1, test018, data)
> +{
> +	int r;
> +	int32_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_int(section, "zero", &n, 600);
> +
> +	ZUC_ASSERT_EQ(0, r);
> +	ZUC_ASSERT_EQ(0, n);
> +	ZUC_ASSERT_EQ(0, errno);
> +}
> +
> +ZUC_TEST_F(config_test_t1, test019, 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_uint(section, "zero", &n, 600);
> +
> +	ZUC_ASSERT_EQ(0, r);
> +	ZUC_ASSERT_EQ(0, n);
> +	ZUC_ASSERT_EQ(0, 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