[PATCH wayland] wl_strtol and wl_strtoul utility functions are added (inlined patch)

[Giulio Camuffo]

2014-10-15 22:01 GMT+03:00 Imran Zaman <imran.zaman at gmail.com>:
> The reason is that strtol is used at many places in weston/wayland..
> and its not covering all the error cases everywhere.. so its better to
> encapsulate it in a function which i did..

You may have a point here, but imho it must not be an exported
function. Put the declaration in wayland-private.h, and just copy it
to weston if you want to have it there too.


--
Giulio

>
> On Wed, Oct 15, 2014 at 9:31 PM, Jasper St. Pierre
> <jstpierre at mecheye.net> wrote:
>> Why? What's the rationale for this?
>>
>> On Wed, Oct 15, 2014 at 6:18 AM, Imran Zaman <imran.zaman at gmail.com> wrote:
>>>
>>> Hi
>>>
>>> The patch is used to replace strtol and strtoul with wl_strtol and
>>> wl_strtoul with inputs and result checks.
>>>
>>> The utility functions are used extensively in wayland and weston so added
>>> appropriate
>>> input and output checks; test cases are also updated; will push the patch
>>> for weston as well.
>>> ----
>>>
>>>
>>> diff --git a/src/scanner.c b/src/scanner.c
>>> index 809130b..3e30fe7 100644
>>> --- a/src/scanner.c
>>> +++ b/src/scanner.c
>>> @@ -315,7 +315,6 @@ start_element(void *data, const char *element_name,
>>> const char **atts)
>>>   struct description *description;
>>>   const char *name, *type, *interface_name, *value, *summary, *since;
>>>   const char *allow_null;
>>> - char *end;
>>>   int i, version;
>>>
>>>   ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
>>> @@ -404,9 +403,7 @@ start_element(void *data, const char *element_name,
>>> const char **atts)
>>>   message->destructor = 0;
>>>
>>>   if (since != NULL) {
>>> - errno = 0;
>>> - version = strtol(since, &end, 0);
>>> - if (errno == EINVAL || end == since || *end != '\0')
>>> + if (!wl_strtol(since, NULL, 0, &version))
>>>   fail(&ctx->loc,
>>>       "invalid integer (%s)\n", since);
>>>   } else {
>>> diff --git a/src/wayland-client.c b/src/wayland-client.c
>>> index b0f77b9..1229b5f 100644
>>> --- a/src/wayland-client.c
>>> +++ b/src/wayland-client.c
>>> @@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
>>>  WL_EXPORT struct wl_display *
>>>  wl_display_connect(const char *name)
>>>  {
>>> - char *connection, *end;
>>> + char *connection;
>>>   int flags, fd;
>>>
>>>   connection = getenv("WAYLAND_SOCKET");
>>>   if (connection) {
>>> - fd = strtol(connection, &end, 0);
>>> - if (*end != '\0')
>>> + if (!wl_strtol(connection, NULL, 0, &fd))
>>>   return NULL;
>>>
>>>   flags = fcntl(fd, F_GETFD);
>>> diff --git a/src/wayland-util.c b/src/wayland-util.c
>>> index b099882..f8267f3 100644
>>> --- a/src/wayland-util.c
>>> +++ b/src/wayland-util.c
>>> @@ -26,6 +26,9 @@
>>>  #include <stdio.h>
>>>  #include <string.h>
>>>  #include <stdarg.h>
>>> +#include <errno.h>
>>> +#include <limits.h>
>>> +#include <stdlib.h>
>>>
>>>  #include "wayland-util.h"
>>>  #include "wayland-private.h"
>>> @@ -361,6 +364,42 @@ wl_map_for_each(struct wl_map *map,
>>> wl_iterator_func_t func, void *data)
>>>   for_each_helper(&map->server_entries, func, data);
>>>  }
>>>
>>> +WL_EXPORT int
>>> +wl_strtol(const char *str, char **endptr, int base, int32_t *val)
>>> +{
>>> + char *end = NULL;
>>> + long v;
>>> +
>>> + if (!str || !val) return 0;
>>> + if (!endptr) endptr = &end;
>>> +
>>> + errno = 0;
>>> + v = strtol(str, endptr, base);
>>> + if (errno != 0 || *endptr == str || **endptr != '\0')
>>> + return 0;
>>> +
>>> + *val = v;
>>> + return 1;
>>> +}
>>> +
>>> +WL_EXPORT int
>>> +wl_strtoul(const char *str, char **endptr, int base, uint32_t *val)
>>> +{
>>> + char *end = NULL;
>>> + unsigned long v;
>>> +
>>> + if (!str || !val) return 0;
>>> + if (!endptr) endptr = &end;
>>> +
>>> + errno = 0;
>>> + v = strtoul(str, endptr, base);
>>> + if (errno != 0 || *endptr == str || **endptr != '\0')
>>> + return 0;
>>> +
>>> + *val = v;
>>> + return 1;
>>> +}
>>> +
>>>  static void
>>>  wl_log_stderr_handler(const char *fmt, va_list arg)
>>>  {
>>> diff --git a/src/wayland-util.h b/src/wayland-util.h
>>> index fd32826..b77d4e3 100644
>>> --- a/src/wayland-util.h
>>> +++ b/src/wayland-util.h
>>> @@ -243,6 +243,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
>>>   return i * 256;
>>>  }
>>>
>>> +int wl_strtol(const char *str, char **endptr, int base, int32_t *val);
>>> +int wl_strtoul(const char *str, char **endptr, int base, uint32_t *val);
>>> +
>>>  /**
>>>   * \brief A union representing all of the basic data types that can be
>>> passed
>>>   * along the wayland wire format.
>>> diff --git a/tests/fixed-test.c b/tests/fixed-test.c
>>> index 739a3b1..349cc48 100644
>>> --- a/tests/fixed-test.c
>>> +++ b/tests/fixed-test.c
>>> @@ -88,3 +88,61 @@ TEST(fixed_int_conversions)
>>>   i = wl_fixed_to_int(f);
>>>   assert(i == -0x50);
>>>  }
>>> +
>>> +TEST(strtol_conversions)
>>> +{
>>> + int ret;
>>> + int32_t val = -1;
>>> + char *end = NULL;
>>> +
>>> + char *str = "12";
>>> + ret = wl_strtol(str, NULL, 10, &val);
>>> + assert(ret == 1);
>>> + assert(val == 12);
>>> +
>>> + ret = wl_strtol(str, &end, 10, &val);
>>> + assert(end != NULL);
>>> + assert(*end == '\0');
>>> +
>>> + str = "s12"; val = -1;
>>> + ret = wl_strtol(str, NULL, 10, &val);
>>> + assert(ret == 0);
>>> + assert(val == -1);
>>> +
>>> + ret = wl_strtol(str, &end, 10, &val);
>>> + assert(end == str);
>>> +
>>> + str = ""; val = -1;
>>> + ret = wl_strtol(str, NULL, 10, &val);
>>> + assert(ret == 0);
>>> + assert(val == -1);
>>> +}
>>> +
>>> +TEST(strtoul_conversions)
>>> +{
>>> + int ret;
>>> + uint32_t val = 0;
>>> + char *end = NULL;
>>> +
>>> + char *str = "15";
>>> + ret = wl_strtoul(str, NULL, 10, &val);
>>> + assert(ret == 1);
>>> + assert(val == 15);
>>> +
>>> + ret = wl_strtoul(str, &end, 10, &val);
>>> + assert(end != NULL);
>>> + assert(*end == '\0');
>>> +
>>> + str = "s15"; val = 0;
>>> + ret = wl_strtoul(str, NULL, 10, &val);
>>> + assert(ret == 0);
>>> + assert(val == 0);
>>> +
>>> + ret = wl_strtoul(str, &end, 10, &val);
>>> + assert(end == str);
>>> +
>>> + str = ""; val = 0;
>>> + ret = wl_strtoul(str, NULL, 10, &val);
>>> + assert(ret == 0);
>>> + assert(val == 0);
>>> +}
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>
>>
>>
>> --
>>   Jasper
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel