[v2] wayland-util: added wl_strtol/wl_strtoul utility functions

Submitted by Imran Zaman on Oct. 16, 2014, 4:11 p.m.

Details

Message ID 1413475906-23073-1-git-send-email-imran.zaman@gmail.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Imran Zaman Oct. 16, 2014, 4:11 p.m.
strtol/strtoul utility functions are used extensively in
weston/wayland, and are not bug-free in their current form.
To avoid definition in weston and wayland, its wrapped
in functions with appropriate input and output checks.
Test cases are also updated.

Signed-off-by: Imran Zaman <imran.zaman@gmail.com>
---
 src/scanner.c        |   5 +--
 src/wayland-client.c |   5 +--
 src/wayland-util.c   |  38 ++++++++++++++++
 src/wayland-util.h   |   4 ++
 tests/fixed-test.c   | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 167 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/scanner.c b/src/scanner.c
index 809130b..165b12b 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, (long *)&version))
 				fail(&ctx->loc,
 				     "invalid integer (%s)\n", since);
 		} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..fd98705 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, (long *)&fd))
 			return NULL;
 
 		flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..dfd2eb1 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,8 @@ 
 #include <stdio.h>
 #include <string.h>
 #include <stdarg.h>
+#include <errno.h>
+#include <limits.h>
 
 #include "wayland-util.h"
 #include "wayland-private.h"
@@ -361,6 +363,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 bool
+wl_strtol(const char *str, char **endptr, int base, long *val)
+{
+	char *end = NULL;
+	long v;
+
+	if (!str || !val) return false;
+	if (!endptr) endptr = &end;
+
+	errno = 0;
+	v = strtol(str, endptr, base);
+	if (errno != 0 || *endptr == str || **endptr != '\0')
+		return false;
+
+	*val = v;
+	return true;
+}
+
+WL_EXPORT bool
+wl_strtoul(const char *str, char **endptr, int base, unsigned long *val)
+{
+	char *end = NULL;
+	unsigned long v;
+
+	if (!str || !val) return false;
+	if (!endptr) endptr = &end;
+
+	errno = 0;
+	v = strtoul(str, endptr, base);
+	if (errno != 0 || *endptr == str || **endptr != '\0')
+		return false;
+
+	*val = v;
+	return true;
+}
+
 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..c66cc41 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -36,6 +36,7 @@  extern "C" {
 #include <stddef.h>
 #include <inttypes.h>
 #include <stdarg.h>
+#include <stdbool.h>
 
 /* GCC visibility */
 #if defined(__GNUC__) && __GNUC__ >= 4
@@ -243,6 +244,9 @@  static inline wl_fixed_t wl_fixed_from_int(int i)
 	return i * 256;
 }
 
+bool wl_strtol(const char *str, char **endptr, int base, long *val);
+bool wl_strtoul(const char *str, char **endptr, int base, unsigned long *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..aa0340c 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,125 @@  TEST(fixed_int_conversions)
 	i = wl_fixed_to_int(f);
 	assert(i == -0x50);
 }
+
+TEST(strtol_conversions)
+{
+	bool ret;
+	long val = -1;
+	char *end = NULL;
+
+	ret = wl_strtol(NULL, NULL, 10, &val);
+	assert(ret == false);
+	assert(val == -1);
+
+	ret = wl_strtol(NULL, NULL, 10, NULL);
+	assert(ret == false);
+
+	char *str = "12";
+	ret = wl_strtol(str, NULL, 10, &val);
+	assert(ret == true);
+	assert(val == 12);
+
+	ret = wl_strtol(str, &end, 10, &val);
+	assert(end != NULL);
+	assert(*end == '\0');
+
+	str = "-12"; val = -1;
+	ret = wl_strtol(str, &end, 10, &val);
+	assert(ret == true);
+	assert(val == -12);
+
+	str = "0x12"; val = -1;
+	ret = wl_strtol(str, &end, 16, &val);
+	assert(ret == true);
+	assert(val == 0x12);
+
+	str = "A"; val = -1;
+	ret = wl_strtol(str, &end, 16, &val);
+	assert(ret == true);
+	assert(val == 10);
+
+	str = "-0x20"; val = -1;
+	ret = wl_strtol(str, &end, 16, &val);
+	assert(ret == true);
+	assert(val == -0x20);
+
+	str = "0012"; val = -1;
+	ret = wl_strtol(str, &end, 8, &val);
+	assert(ret == true);
+	assert(val == 10);
+
+	str = "0101"; val = -1;
+	ret = wl_strtol(str, &end, 2, &val);
+	assert(ret == true);
+	assert(val == 5);
+
+	str = "s12"; val = -1;
+	ret = wl_strtol(str, NULL, 10, &val);
+	assert(ret == false);
+	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 == false);
+	assert(val == -1);
+}
+
+TEST(strtoul_conversions)
+{
+	bool ret;
+	unsigned long val = 0;
+	char *end = NULL;
+
+	ret = wl_strtoul(NULL, NULL, 10, &val);
+	assert(ret == false);
+	assert(val == 0);
+
+	ret = wl_strtoul(NULL, NULL, 10, NULL);
+	assert(ret == false);
+
+	char *str = "15";
+	ret = wl_strtoul(str, NULL, 10, &val);
+	assert(ret == true);
+	assert(val == 15);
+
+	ret = wl_strtoul(str, &end, 10, &val);
+	assert(end != NULL);
+	assert(*end == '\0');
+
+	str = "0x12"; val = 0;
+	ret = wl_strtoul(str, &end, 16, &val);
+	assert(ret == true);
+	assert(val == 18);
+
+	str = "A"; val = 0;
+	ret = wl_strtoul(str, &end, 16, &val);
+	assert(ret == true);
+	assert(val == 10);
+
+	str = "0012"; val = 0;
+	ret = wl_strtoul(str, &end, 8, &val);
+	assert(ret == true);
+	assert(val == 10);
+
+	str = "0101"; val = 0;
+	ret = wl_strtoul(str, &end, 2, &val);
+	assert(ret == true);
+	assert(val == 5);
+
+	str = "s15"; val = 0;
+	ret = wl_strtoul(str, NULL, 10, &val);
+	assert(ret == false);
+	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 == false);
+	assert(val == 0);
+}

Comments

Hi,

Personally, I'd rather see these functions private (somebody has already
mentioned that),
but I understand there are reasons for them to be public and maybe in the
end it will have more pros..
Anyway, I have few nitpicks and a questions - see below.

On 16 October 2014 18:11, Imran Zaman <imran.zaman@gmail.com> wrote:

> strtol/strtoul utility functions are used extensively in
> weston/wayland, and are not bug-free in their current form.
> To avoid definition in weston and wayland, its wrapped
> in functions with appropriate input and output checks.
> Test cases are also updated.
>
> Signed-off-by: Imran Zaman <imran.zaman@gmail.com>
> ---
>  src/scanner.c        |   5 +--
>  src/wayland-client.c |   5 +--
>  src/wayland-util.c   |  38 ++++++++++++++++
>  src/wayland-util.h   |   4 ++
>  tests/fixed-test.c   | 122
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 167 insertions(+), 7 deletions(-)
>
> diff --git a/src/scanner.c b/src/scanner.c
> index 809130b..165b12b 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, (long *)&version))
>                                 fail(&ctx->loc,
>                                      "invalid integer (%s)\n", since);
>                 } else {
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index b0f77b9..fd98705 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, (long *)&fd))
>                         return NULL;
>
>                 flags = fcntl(fd, F_GETFD);
> diff --git a/src/wayland-util.c b/src/wayland-util.c
> index b099882..dfd2eb1 100644
> --- a/src/wayland-util.c
> +++ b/src/wayland-util.c
> @@ -26,6 +26,8 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <stdarg.h>
> +#include <errno.h>
> +#include <limits.h>
>
>  #include "wayland-util.h"
>  #include "wayland-private.h"
> @@ -361,6 +363,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 bool
> +wl_strtol(const char *str, char **endptr, int base, long *val)
> +{
> +       char *end = NULL;
> +       long v;
> +
> +       if (!str || !val) return false;
> +       if (!endptr) endptr = &end;
>

The 'return false' and 'endptr =&end' should be on new line
http://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n33


> +
> +       errno = 0;
> +       v = strtol(str, endptr, base);
> +       if (errno != 0 || *endptr == str || **endptr != '\0')
> +               return false;
> +
> +       *val = v;
> +       return true;
> +}
> +
> +WL_EXPORT bool
> +wl_strtoul(const char *str, char **endptr, int base, unsigned long *val)
> +{
> +       char *end = NULL;
> +       unsigned long v;
> +
> +       if (!str || !val) return false;
> +       if (!endptr) endptr = &end;
>

The same


> +
> +       errno = 0;
> +       v = strtoul(str, endptr, base);
> +       if (errno != 0 || *endptr == str || **endptr != '\0')
> +               return false;
> +
> +       *val = v;
> +       return true;
> +}
> +
>  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..c66cc41 100644
> --- a/src/wayland-util.h
> +++ b/src/wayland-util.h
> @@ -36,6 +36,7 @@ extern "C" {
>  #include <stddef.h>
>  #include <inttypes.h>
>  #include <stdarg.h>
> +#include <stdbool.h>
>
>  /* GCC visibility */
>  #if defined(__GNUC__) && __GNUC__ >= 4
> @@ -243,6 +244,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
>         return i * 256;
>  }
>
> +bool wl_strtol(const char *str, char **endptr, int base, long *val);
> +bool wl_strtoul(const char *str, char **endptr, int base, unsigned long
> *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..aa0340c 100644
> --- a/tests/fixed-test.c
> +++ b/tests/fixed-test.c
> @@ -88,3 +88,125 @@ TEST(fixed_int_conversions)
>         i = wl_fixed_to_int(f);
>         assert(i == -0x50);
>  }
> +
> +TEST(strtol_conversions)
> +{
> +       bool ret;
> +       long val = -1;
> +       char *end = NULL;
> +
> +       ret = wl_strtol(NULL, NULL, 10, &val);
> +       assert(ret == false);
> +       assert(val == -1);
> +
> +       ret = wl_strtol(NULL, NULL, 10, NULL);
> +       assert(ret == false);
> +
> +       char *str = "12";
>

This variable should be declared on the beginning of the function and only
assigned here
(again from the Contributing document).


> +       ret = wl_strtol(str, NULL, 10, &val);
> +       assert(ret == true);
> +       assert(val == 12);
> +
> +       ret = wl_strtol(str, &end, 10, &val);
> +       assert(end != NULL);
> +       assert(*end == '\0');
> +
> +       str = "-12"; val = -1;
> +       ret = wl_strtol(str, &end, 10, &val);
> +       assert(ret == true);
> +       assert(val == -12);
> +
> +       str = "0x12"; val = -1;
> +       ret = wl_strtol(str, &end, 16, &val);
> +       assert(ret == true);
> +       assert(val == 0x12);
> +
> +       str = "A"; val = -1;
> +       ret = wl_strtol(str, &end, 16, &val);
> +       assert(ret == true);
> +       assert(val == 10);
> +
> +       str = "-0x20"; val = -1;
> +       ret = wl_strtol(str, &end, 16, &val);
> +       assert(ret == true);
> +       assert(val == -0x20);
> +
> +       str = "0012"; val = -1;
> +       ret = wl_strtol(str, &end, 8, &val);
> +       assert(ret == true);
> +       assert(val == 10);
> +
> +       str = "0101"; val = -1;
> +       ret = wl_strtol(str, &end, 2, &val);
> +       assert(ret == true);
> +       assert(val == 5);
> +
> +       str = "s12"; val = -1;
> +       ret = wl_strtol(str, NULL, 10, &val);
> +       assert(ret == false);
> +       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 == false);
> +       assert(val == -1);
> +}
> +
> +TEST(strtoul_conversions)
> +{
> +       bool ret;
> +       unsigned long val = 0;
> +       char *end = NULL;
> +
> +       ret = wl_strtoul(NULL, NULL, 10, &val);
> +       assert(ret == false);
> +       assert(val == 0);
> +
> +       ret = wl_strtoul(NULL, NULL, 10, NULL);
> +       assert(ret == false);
> +
> +       char *str = "15";
>

the same


> +       ret = wl_strtoul(str, NULL, 10, &val);
> +       assert(ret == true);
> +       assert(val == 15);
> +
> +       ret = wl_strtoul(str, &end, 10, &val);
> +       assert(end != NULL);
> +       assert(*end == '\0');
> +
> +       str = "0x12"; val = 0;
> +       ret = wl_strtoul(str, &end, 16, &val);
> +       assert(ret == true);
> +       assert(val == 18);
> +
> +       str = "A"; val = 0;
> +       ret = wl_strtoul(str, &end, 16, &val);
> +       assert(ret == true);
> +       assert(val == 10);
> +
> +       str = "0012"; val = 0;
> +       ret = wl_strtoul(str, &end, 8, &val);
> +       assert(ret == true);
> +       assert(val == 10);
> +
> +       str = "0101"; val = 0;
> +       ret = wl_strtoul(str, &end, 2, &val);
> +       assert(ret == true);
> +       assert(val == 5);
> +
> +       str = "s15"; val = 0;
> +       ret = wl_strtoul(str, NULL, 10, &val);
> +       assert(ret == false);
> +       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 == false);
> +       assert(val == 0);
> +}
> --
> 1.9.1
>

What I'm wondering is what should be the behavior of wl_strtoul for
negative numbers?
strtoul silently converts them, but I don't think this is what we always
want... or is it?
And also some tests for overflow would be handy.

Regards,
Marek


>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
I pushed an updated patch v3. Added test cases for overflow and also
check for negative numbers for wl_strtoul..
please review

BR
imran

On Wed, Oct 22, 2014 at 12:13 PM, Marek Chalupa <mchqwerty@gmail.com> wrote:
> Hi,
>
> Personally, I'd rather see these functions private (somebody has already
> mentioned that),
> but I understand there are reasons for them to be public and maybe in the
> end it will have more pros..
> Anyway, I have few nitpicks and a questions - see below.
>
> On 16 October 2014 18:11, Imran Zaman <imran.zaman@gmail.com> wrote:
>>
>> strtol/strtoul utility functions are used extensively in
>> weston/wayland, and are not bug-free in their current form.
>> To avoid definition in weston and wayland, its wrapped
>> in functions with appropriate input and output checks.
>> Test cases are also updated.
>>
>> Signed-off-by: Imran Zaman <imran.zaman@gmail.com>
>> ---
>>  src/scanner.c        |   5 +--
>>  src/wayland-client.c |   5 +--
>>  src/wayland-util.c   |  38 ++++++++++++++++
>>  src/wayland-util.h   |   4 ++
>>  tests/fixed-test.c   | 122
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 167 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index 809130b..165b12b 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, (long *)&version))
>>                                 fail(&ctx->loc,
>>                                      "invalid integer (%s)\n", since);
>>                 } else {
>> diff --git a/src/wayland-client.c b/src/wayland-client.c
>> index b0f77b9..fd98705 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, (long *)&fd))
>>                         return NULL;
>>
>>                 flags = fcntl(fd, F_GETFD);
>> diff --git a/src/wayland-util.c b/src/wayland-util.c
>> index b099882..dfd2eb1 100644
>> --- a/src/wayland-util.c
>> +++ b/src/wayland-util.c
>> @@ -26,6 +26,8 @@
>>  #include <stdio.h>
>>  #include <string.h>
>>  #include <stdarg.h>
>> +#include <errno.h>
>> +#include <limits.h>
>>
>>  #include "wayland-util.h"
>>  #include "wayland-private.h"
>> @@ -361,6 +363,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 bool
>> +wl_strtol(const char *str, char **endptr, int base, long *val)
>> +{
>> +       char *end = NULL;
>> +       long v;
>> +
>> +       if (!str || !val) return false;
>> +       if (!endptr) endptr = &end;
>
>
> The 'return false' and 'endptr =&end' should be on new line
> http://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n33
>
>>
>> +
>> +       errno = 0;
>> +       v = strtol(str, endptr, base);
>> +       if (errno != 0 || *endptr == str || **endptr != '\0')
>> +               return false;
>> +
>> +       *val = v;
>> +       return true;
>> +}
>> +
>> +WL_EXPORT bool
>> +wl_strtoul(const char *str, char **endptr, int base, unsigned long *val)
>> +{
>> +       char *end = NULL;
>> +       unsigned long v;
>> +
>> +       if (!str || !val) return false;
>> +       if (!endptr) endptr = &end;
>
>
> The same
>
>>
>> +
>> +       errno = 0;
>> +       v = strtoul(str, endptr, base);
>> +       if (errno != 0 || *endptr == str || **endptr != '\0')
>> +               return false;
>> +
>> +       *val = v;
>> +       return true;
>> +}
>> +
>>  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..c66cc41 100644
>> --- a/src/wayland-util.h
>> +++ b/src/wayland-util.h
>> @@ -36,6 +36,7 @@ extern "C" {
>>  #include <stddef.h>
>>  #include <inttypes.h>
>>  #include <stdarg.h>
>> +#include <stdbool.h>
>>
>>  /* GCC visibility */
>>  #if defined(__GNUC__) && __GNUC__ >= 4
>> @@ -243,6 +244,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
>>         return i * 256;
>>  }
>>
>> +bool wl_strtol(const char *str, char **endptr, int base, long *val);
>> +bool wl_strtoul(const char *str, char **endptr, int base, unsigned long
>> *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..aa0340c 100644
>> --- a/tests/fixed-test.c
>> +++ b/tests/fixed-test.c
>> @@ -88,3 +88,125 @@ TEST(fixed_int_conversions)
>>         i = wl_fixed_to_int(f);
>>         assert(i == -0x50);
>>  }
>> +
>> +TEST(strtol_conversions)
>> +{
>> +       bool ret;
>> +       long val = -1;
>> +       char *end = NULL;
>> +
>> +       ret = wl_strtol(NULL, NULL, 10, &val);
>> +       assert(ret == false);
>> +       assert(val == -1);
>> +
>> +       ret = wl_strtol(NULL, NULL, 10, NULL);
>> +       assert(ret == false);
>> +
>> +       char *str = "12";
>
>
> This variable should be declared on the beginning of the function and only
> assigned here
> (again from the Contributing document).
>
>>
>> +       ret = wl_strtol(str, NULL, 10, &val);
>> +       assert(ret == true);
>> +       assert(val == 12);
>> +
>> +       ret = wl_strtol(str, &end, 10, &val);
>> +       assert(end != NULL);
>> +       assert(*end == '\0');
>> +
>> +       str = "-12"; val = -1;
>> +       ret = wl_strtol(str, &end, 10, &val);
>> +       assert(ret == true);
>> +       assert(val == -12);
>> +
>> +       str = "0x12"; val = -1;
>> +       ret = wl_strtol(str, &end, 16, &val);
>> +       assert(ret == true);
>> +       assert(val == 0x12);
>> +
>> +       str = "A"; val = -1;
>> +       ret = wl_strtol(str, &end, 16, &val);
>> +       assert(ret == true);
>> +       assert(val == 10);
>> +
>> +       str = "-0x20"; val = -1;
>> +       ret = wl_strtol(str, &end, 16, &val);
>> +       assert(ret == true);
>> +       assert(val == -0x20);
>> +
>> +       str = "0012"; val = -1;
>> +       ret = wl_strtol(str, &end, 8, &val);
>> +       assert(ret == true);
>> +       assert(val == 10);
>> +
>> +       str = "0101"; val = -1;
>> +       ret = wl_strtol(str, &end, 2, &val);
>> +       assert(ret == true);
>> +       assert(val == 5);
>> +
>> +       str = "s12"; val = -1;
>> +       ret = wl_strtol(str, NULL, 10, &val);
>> +       assert(ret == false);
>> +       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 == false);
>> +       assert(val == -1);
>> +}
>> +
>> +TEST(strtoul_conversions)
>> +{
>> +       bool ret;
>> +       unsigned long val = 0;
>> +       char *end = NULL;
>> +
>> +       ret = wl_strtoul(NULL, NULL, 10, &val);
>> +       assert(ret == false);
>> +       assert(val == 0);
>> +
>> +       ret = wl_strtoul(NULL, NULL, 10, NULL);
>> +       assert(ret == false);
>> +
>> +       char *str = "15";
>
>
> the same
>
>>
>> +       ret = wl_strtoul(str, NULL, 10, &val);
>> +       assert(ret == true);
>> +       assert(val == 15);
>> +
>> +       ret = wl_strtoul(str, &end, 10, &val);
>> +       assert(end != NULL);
>> +       assert(*end == '\0');
>> +
>> +       str = "0x12"; val = 0;
>> +       ret = wl_strtoul(str, &end, 16, &val);
>> +       assert(ret == true);
>> +       assert(val == 18);
>> +
>> +       str = "A"; val = 0;
>> +       ret = wl_strtoul(str, &end, 16, &val);
>> +       assert(ret == true);
>> +       assert(val == 10);
>> +
>> +       str = "0012"; val = 0;
>> +       ret = wl_strtoul(str, &end, 8, &val);
>> +       assert(ret == true);
>> +       assert(val == 10);
>> +
>> +       str = "0101"; val = 0;
>> +       ret = wl_strtoul(str, &end, 2, &val);
>> +       assert(ret == true);
>> +       assert(val == 5);
>> +
>> +       str = "s15"; val = 0;
>> +       ret = wl_strtoul(str, NULL, 10, &val);
>> +       assert(ret == false);
>> +       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 == false);
>> +       assert(val == 0);
>> +}
>> --
>> 1.9.1
>
>
> What I'm wondering is what should be the behavior of wl_strtoul for negative
> numbers?
> strtoul silently converts them, but I don't think this is what we always
> want... or is it?
> And also some tests for overflow would be handy.
>
> Regards,
> Marek
>
>>
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>
On 10/22/2014 02:13 AM, Marek Chalupa wrote:

> What I'm wondering is what should be the behavior of wl_strtoul for
> negative numbers?
> strtoul silently converts them, but I don't think this is what we always
> want... or is it?

-1 is a handy shortcut for strtoul to get all the bits turned on and get 
the maximum possible value.

Not sure if other negative numbers are much use though.