On Thu, Jul 07, 2016 at 02:24:39PM -0700, Bryce Harrington wrote:
> 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 <br...@osg.samsung.com>
>
> If this approach looks ok to folks, there are other irregular uses of
> strto*l I'll clean up as well subsequently.
it'd be best to have a safe_strtol helper function in a weston-util.h
header and just enforce use of that. because most of the time all we care
about is "has it parsed and what's the result", you can get that with a
bool safe_strtol(const char *string, long *result);
or safe_atoi, or whatever you want to call it.
Cheers,
Peter
>
> > ---
> > 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') {
> > *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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
