Re: [PATCH v1] Added string conversion utility functions
Interesting read.. have some arguments about the raised points.. but
lets not go in that direction..
Can you guys suggest to do some changes or shall i drop the patch
altogether? if you are happy with current implementation lets live
with it.. I noticed bug when weston was not working properly due to
improper error handling of strtol in one place, so I thought it would
be good idea to fix it wherever it is used (and hence this patch) but
seems that it is not always the case :-)
BR
imran
p.s: To me it was really simple to understand that in MOST (=NOT ALL)
of the cases str-to-number conversion was done using strtol/strtoul
but without proper error handling. I moved it to single place to make
the life easier IMHO...
On Mon, Dec 1, 2014 at 8:30 PM, Bill Spitzak spit...@gmail.com wrote:
On 12/01/2014 04:10 AM, Pekka Paalanen wrote:
other = strtol(pid, end, 0);
if (end != pid + 10) {
weston_log(can't parse lock file %s\n,
lockfile);
close(fd);
errno = EEXIST;
return -1;
}
'pid' is a fixed size string read in from the lock file, which is
converted into a number of type pid_t. Because the number is assumed to
be printed by %10d\n, the file should have at least 11 bytes
available, and we assume all the 10 characters form a valid number
(with leading spaces). It's all just a way to avoid dealing with
unexpected EOF when reading the file, and to avoid not knowing in
advance how many characters long the number is.
This code still wants to parse the whole string as a single number, but
it also knows the number will end in a newline instead of nul. It
wouldn't be difficult to replace that newline with nul before parsing,
if you really want to convert this code to use a helper. But while
doing so, you have to ask yourself: does this actually make the code
any easier to understand or more correct?
I believe you are correct, and this is a good indication that blindly
inserting the replacement function is not a good idea.
The original code failed if there was a digit at offset 11 (as well as other
reasons for failing). The proposed replacement failed if offset 11 was
anything other than NUL. This is different. When I said the endptr was not
needed, my proposal actually has the exact same mistake.
This does bring up a question as to whether the helper function should eat
trailing whitespace.
But also that you can't drop the wrapper in everywhere.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On Tue, 2 Dec 2014 15:45:40 +0200
Imran Zaman imran.za...@gmail.com wrote:
Interesting read.. have some arguments about the raised points.. but
lets not go in that direction..
Can you guys suggest to do some changes or shall i drop the patch
altogether? if you are happy with current implementation lets live
with it.. I noticed bug when weston was not working properly due to
improper error handling of strtol in one place, so I thought it would
be good idea to fix it wherever it is used (and hence this patch) but
seems that it is not always the case :-)
BR
imran
p.s: To me it was really simple to understand that in MOST (=NOT ALL)
of the cases str-to-number conversion was done using strtol/strtoul
but without proper error handling. I moved it to single place to make
the life easier IMHO...
Hi,
fixing bugs is always welcome and recommended.
If you see a place, where the error checking is wanted/attempted but
insufficient, it would be very nice to fix it.
However, writing a helper function whose benefit over a direct call to
strto*() is only the handling of errno, it might not be worth it in my
opinion. Such a case is probably simpler to just fix in place than make
it use a helper function. (This might change with libweston, though.)
If the helper can assume that the whole given string must be a valid
number, I think that would be much more worth it, because it would then
hide the end pointer and nul checks. Such a function would be simple to
use and make the code where it is used much more obvious.
If you want to add error checking to places where there is none to
begin with, I think that would be very good, but also should be a
separate patch.
I don't see any value in adding error checking (by calling a helper)
and then ignoring the check result though.
It depends on the whole, really, where we strike the balance between
generality and simplicity of the helper function API.
I think a good next proposal would be to keep your helper functions
mostly as is (they are quite thorough), and replace only the call sites
where there already is some error checking.
Another thing is the return value: your helper functions return success
only if the whole string is a valid number (ends in nul). It does not
discriminate between conversion errors and converting only a substring
successfully. That makes it hard to use for anything where you want to
parse only a substring and stop at the first non-digit character. The
caller will not know if the number overflowed or if it just found a
non-digit character. If a non-digit character is expected, the return
value success/fail is completely useless and there is no way to even
manually check errno to see if the conversion failed or not. I think
this is one thing that Bill tried to explain. This is why the end_ptr
argument is useless as it is, because on success it can only point to
the end of the string.
To make it useful, the success/fail return value needs to be a little
different.
We have two basic use cases:
a) convert a whole string (up to nul-terminator), and
b) convert possibly a substring (up to non-digit or nul) and tell where
the conversion stopped
If you want to make a helper function that is useful for both at the
same time, you need to return success when the conversion succeeds,
even if it does not end in nul. OTOH, if the caller wants to verify the
whole string up to nul, it is not interested in end_ptr, which means
that you could check the nul iff end_ptr is not wanted and save the
effort from the caller.
Then there is the question of negative numbers which you special-cased
in convert_strtoul(). I'd rather it followed the stroul() semantics to
avoid surprises.
When you do return failure like in weston_strtoi(), you should make
sure errno communicates the error like strtol() would. That's part of
consistency.
Finally, you should decide how the equivalent of an empty string parses:
is it a failure or does it produce zero? I think you need to look at
what majority of the potential callers would expect, and go with that.
For instance, in multi-resource.c create_device(), the string to be
parsed is A:B. If substring A is actually empty, it leads to the
value zero. I don't know if that is an intentional shorthand, but I
think it works.
Writing tests for the helpers is excellent. The cherry on top would be
documentation.
Thanks,
pq
On Mon, Dec 1, 2014 at 8:30 PM, Bill Spitzak spit...@gmail.com wrote:
On 12/01/2014 04:10 AM, Pekka Paalanen wrote:
other = strtol(pid, end, 0);
if (end != pid + 10) {
weston_log(can't parse lock file %s\n,
lockfile);
close(fd);
errno = EEXIST;
return -1;
}
'pid' is a fixed size string read in from the lock file, which is
converted into a number of type pid_t. Because the number is assumed to
be
Re: [PATCH v1] Added string conversion utility functions
I think the desired function was to fail if there was text after the number, fail on blank strings, and fail on overflow. All of these are somewhat cryptic with strtol. However, writing a helper function whose benefit over a direct call to strto*() is only the handling of errno, it might not be worth it in my opinion. Such a case is probably simpler to just fix in place than make it use a helper function. (This might change with libweston, though.) If the helper can assume that the whole given string must be a valid number, I think that would be much more worth it, because it would then hide the end pointer and nul checks. Such a function would be simple to use and make the code where it is used much more obvious. That I believe was the original intent. For that reason it should not return endptr, as that is useless. If existing code is using endptr that indicates it cannot use the replacement function. The function should do some/all of the following, all of which are clumsy to do with the library functions, and are often attempted by the calling code in sometimes-cryptic ways: - Fail if there is non-whitespace after the number - Fail without crashing if the string pointer is null - Fail if there is no number (all whitespace or zero-length string) - Fail if there is overflow - Don't change errno if it does not fail - Always set errno non-zero if it does fail, or return a different value for each type of failure and don't change errno. - Don't do octal conversion for leading zero, but allow 0x for hex. Maybe use 0o or something for octal if people want it. - Floating point can parse nan and inf including sign - Floating point accepts hex integers (not hex fractions, that would be nice but is much harder. The main point is to make an existing config file not fail if it uses hex for an integer parameter and that parameter is changed to floating point). ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On Tue, 25 Nov 2014 22:17:24 +0200
Imran Zaman imran.za...@gmail.com wrote:
Thanks Bill for your comments. Plz see my comments inline.
On Tue, Nov 25, 2014 at 9:26 PM, Bill Spitzak spit...@gmail.com wrote:
On 11/24/2014 11:12 PM, Imran Zaman wrote:
On Tue, Nov 25, 2014 at 1:15 AM, Bill Spitzak spit...@gmail.com wrote:
On 11/24/2014 11:32 AM, Imran Zaman wrote:
[IZ2] This is the case where endptr is used in patch. I can remove
endptr but it seems its used so i have to keep the existing
functionality in place.
+ if (!weston_strtoi(pid, end, 0, other) || end != pid + 10)
Use strlen(pid) != 10 instead.
This will not exactly replace the implemented logic e.g.
pid=123abcdefg.
strlen(pid) = 10
end = 3 (end != pid + 10)
So cant use the solution which u propose...
But weston_strtoi will return false in that case.
pid=123456789.
strlen(pid) = 10
end = pid + 10
.. endptr is needed by the caller of the function..
Let's look at what that piece of code originally intends to do:
xwayland/launcher.c
static int
create_lockfile(int display, char *lockfile, size_t lsize)
{
char pid[16], *end;
int fd, size;
pid_t other;
snprintf(lockfile, lsize, /tmp/.X%d-lock, display);
fd = open(lockfile, O_WRONLY | O_CLOEXEC | O_CREAT | O_EXCL, 0444);
if (fd 0 errno == EEXIST) {
fd = open(lockfile, O_CLOEXEC | O_RDONLY);
if (fd 0 || read(fd, pid, 11) != 11) {
weston_log(can't read lock file %s: %s\n,
lockfile, strerror(errno));
if (fd = 0)
close (fd);
errno = EEXIST;
return -1;
}
other = strtol(pid, end, 0);
if (end != pid + 10) {
weston_log(can't parse lock file %s\n,
lockfile);
close(fd);
errno = EEXIST;
return -1;
}
if (kill(other, 0) 0 errno == ESRCH) {
/* stale lock file; unlink and try again */
weston_log(unlinking stale lock file %s\n, lockfile);
close(fd);
if (unlink(lockfile))
/* If we fail to unlink, return EEXIST
so we try the next display number.*/
errno = EEXIST;
else
errno = EAGAIN;
return -1;
}
close(fd);
errno = EEXIST;
return -1;
} else if (fd 0) {
weston_log(failed to create lock file %s: %s\n,
lockfile, strerror(errno));
return -1;
}
/* Subtle detail: we use the pid of the wayland
* compositor, not the xserver in the lock file. */
size = snprintf(pid, sizeof pid, %10d\n, getpid());
if (write(fd, pid, size) != size) {
unlink(lockfile);
close(fd);
return -1;
}
close(fd);
return 0;
}
'pid' is a fixed size string read in from the lock file, which is
converted into a number of type pid_t. Because the number is assumed to
be printed by %10d\n, the file should have at least 11 bytes
available, and we assume all the 10 characters form a valid number
(with leading spaces). It's all just a way to avoid dealing with
unexpected EOF when reading the file, and to avoid not knowing in
advance how many characters long the number is.
This code still wants to parse the whole string as a single number, but
it also knows the number will end in a newline instead of nul. It
wouldn't be difficult to replace that newline with nul before parsing,
if you really want to convert this code to use a helper. But while
doing so, you have to ask yourself: does this actually make the code
any easier to understand or more correct?
If I write the following code:
if (strtol(string, 0, 0, 0)) ...
I want it to crash so I notice that I passed 0 for the val output
pointer.
There is no reason to every pass null here so it must be a programmer
mistake. As you have written it this will act like there is a syntax
error
in string, which could lead a person trying to debug their code down the
wrong path.
Sorry, why is it a programmer mistake and how would it crash? Let me
add the exact test code and will come back to you. It should not crash
for sure IMO.
struct Foo {
int first_member;
int integer;
} Foo;
struct Foo* f(char* text) {
struct Foo* pointer;
pointer = code_that_returns_NULL_unexpectedly();
weston_strtoi(text, 0, 0, (p-integer)); // crash!
return pointer;
Re: [PATCH v1] Added string conversion utility functions
On 12/01/2014 04:10 AM, Pekka Paalanen wrote:
other = strtol(pid, end, 0);
if (end != pid + 10) {
weston_log(can't parse lock file %s\n,
lockfile);
close(fd);
errno = EEXIST;
return -1;
}
'pid' is a fixed size string read in from the lock file, which is
converted into a number of type pid_t. Because the number is assumed to
be printed by %10d\n, the file should have at least 11 bytes
available, and we assume all the 10 characters form a valid number
(with leading spaces). It's all just a way to avoid dealing with
unexpected EOF when reading the file, and to avoid not knowing in
advance how many characters long the number is.
This code still wants to parse the whole string as a single number, but
it also knows the number will end in a newline instead of nul. It
wouldn't be difficult to replace that newline with nul before parsing,
if you really want to convert this code to use a helper. But while
doing so, you have to ask yourself: does this actually make the code
any easier to understand or more correct?
I believe you are correct, and this is a good indication that blindly
inserting the replacement function is not a good idea.
The original code failed if there was a digit at offset 11 (as well as
other reasons for failing). The proposed replacement failed if offset 11
was anything other than NUL. This is different. When I said the endptr
was not needed, my proposal actually has the exact same mistake.
This does bring up a question as to whether the helper function should
eat trailing whitespace.
But also that you can't drop the wrapper in everywhere.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On 11/24/2014 11:12 PM, Imran Zaman wrote:
On Tue, Nov 25, 2014 at 1:15 AM, Bill Spitzak spit...@gmail.com wrote:
On 11/24/2014 11:32 AM, Imran Zaman wrote:
[IZ2] This is the case where endptr is used in patch. I can remove
endptr but it seems its used so i have to keep the existing
functionality in place.
+ if (!weston_strtoi(pid, end, 0, other) || end != pid + 10)
Use strlen(pid) != 10 instead.
This will not exactly replace the implemented logic e.g.
pid=123abcdefg.
strlen(pid) = 10
end = 3 (end != pid + 10)
So cant use the solution which u propose...
But weston_strtoi will return false in that case.
If I write the following code:
if (strtol(string, 0, 0, 0)) ...
I want it to crash so I notice that I passed 0 for the val output pointer.
There is no reason to every pass null here so it must be a programmer
mistake. As you have written it this will act like there is a syntax error
in string, which could lead a person trying to debug their code down the
wrong path.
Sorry, why is it a programmer mistake and how would it crash? Let me
add the exact test code and will come back to you. It should not crash
for sure IMO.
struct Foo {
int first_member;
int integer;
} Foo;
struct Foo* f(char* text) {
struct Foo* pointer;
pointer = code_that_returns_NULL_unexpectedly();
weston_strtoi(text, 0, 0, (p-integer)); // crash!
return pointer;
}
Note that with your version it does not crash if integer is the first
member of struct Foo. What I recommend is making it crash in both cases
by not testing the val pointer and instead trying to write a value to it.
At least one case above is using endptr.. if we can replace it with
exactly similarly logic, I can remove it otherwies I dont see any harm
in its usage
I have looked into that a bit more. The only use of endptr (other than
to replicate the tests that your function does) is in the xpm parser. In
that case I am not sure if it may be depending on getting a zero for
zero-length. It may be best to leave that code calling the original
function rather than risk breaking the parser.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
Thanks Bill for your comments. Please see my comments inline.
On Fri, Nov 21, 2014 at 9:31 PM, Bill Spitzak spit...@gmail.com wrote:
There is no need to pass endptr, because your functions will always fail if
it is not set to point to the terminating null.
[IZ] endptr is passed other than null; please see the whole patch. why
do u think it will fail?
I also suspect that the base is never used. In addition a possible future
fix would be to accept 0x for hex but a leading zero is decimal, not octal,
which means this function would generate the base.
[IZ] Base is used in weston code as well, please see the whole patch.
I am also worried about passing the out ptr. Depending on the sizes of int,
long, and long long this will easily result in code that compiles on one
platform but not on another. Returning the value would avoid this and reduce
the number of functions. Instead pass a pointer to a variable to store the
error into, or use errno?
Would prefer you not test the addresses of out parameters for null. It
should crash at this point, as this is a programming error, not a runtime
error. Returning false will make it hard to figure out what is wrong.
[IZ] can u please elaborate it bit more as I can add more test cases
to cover the corner case if I understand clearly what do you want to
highlight?
It is useful to pass -1 for strtoul and this should continue to work. It is
the only easily-recognized value for largest number possible. May be ok to
not allow other negative numbers however.
[IZ] IMO its good to keep the APIs simple and homogeneous with
appropriate data structures needed for the input
BR
imran
On 11/21/2014 07:38 AM, Imran Zaman wrote:
+static bool
+convert_strtol(const char *str, char **endptr, int base, long *val)
+{
+ char *end = NULL;
+ long v;
+ int prev_errno = errno;
+
+ if (!str || !val)
+ return false;
Please do not do the above tests!
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ errno = prev_errno;
+ *val = v;
+ return true;
+}
+
+static bool
+convert_strtoul (const char *str, char **endptr, int base, unsigned long
*val)
+{
+ char *end = NULL;
+ unsigned long v;
+ int i = 0;
+ int prev_errno = errno;
+
+ if (!str || !val)
+ return false;
+ /* check for negative numbers */
+ while (str[i]) {
+ if (!isspace(str[i])) {
+ if (str[i] == '-')
+ return false;
+ else
+ break;
+ }
+ i++;
+ }
You could do the above test afterwards and check for and allow the -1 value.
+
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ errno = prev_errno;
+ *val = v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtoi(const char *str, char **endptr, int base, int *val)
+{
+ long v;
+
+ if (!convert_strtol(str, endptr, base, v) || v INT_MAX
+ || v INT_MIN)
+ return false;
+
+ *val = (int)v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtol(const char *str, char **endptr, int base, long *val)
+{
+ return convert_strtol(str, endptr, base, val);
+}
+
+WL_EXPORT bool
+weston_strtoui(const char *str, char **endptr, int base, unsigned int
*val)
+{
+ unsigned long v;
+
+ if (!convert_strtoul(str, endptr, base, v) || v UINT_MAX)
+ return false;
+
+ *val = (unsigned int)v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtoul(const char *str, char **endptr, int base, unsigned long
*val)
+{
+ return convert_strtoul(str, endptr, base, val);
+}
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On 11/24/2014 05:08 AM, Imran Zaman wrote: Thanks Bill for your comments. Please see my comments inline. On Fri, Nov 21, 2014 at 9:31 PM, Bill Spitzak spit...@gmail.com wrote: There is no need to pass endptr, because your functions will always fail if it is not set to point to the terminating null. [IZ] endptr is passed other than null; please see the whole patch. why do u think it will fail? What I meant was that if this function returns success, endptr will ALWAYS be set to the string+strlen(string), ie at the NULL. As the caller can easily create this answer itself it seems useless to pass a pointer to store it. Although endptr might not point at the null when this fails, it seems hard for the caller to do anything useful with this. It would have to first determine if the error was due to something other than trailing characters, which would involve replicating the entire function locally. Therefore I think this argument is useless and there is no reason to support it. Also the grep below shows that nobody uses the endptr except to do the error checking that this function is removing the need for. I also suspect that the base is never used. In addition a possible future fix would be to accept 0x for hex but a leading zero is decimal, not octal, which means this function would generate the base. [IZ] Base is used in weston code as well, please see the whole patch. Out of 14 calls (found with git grep strto), 6 use 10, 1 uses 16, and 7 use 0. I believe all the 10s are to avoid the unexpected-octal problem. I can't be absolutely certain, but I suspect the reason this is done is to prevent unexpected octal conversion, and in fact there is no desire to make 0x prefix not work. This is a good indication that perhaps your functions should do this as well. Basically only base 10 and 0x for base 16 work. wscreensaver-glue uses 16 to read the color map out of an xpm file. I cannot find any code in weston that calls this xpm converter. I am also worried about passing the out ptr. Depending on the sizes of int, long, and long long this will easily result in code that compiles on one platform but not on another. Returning the value would avoid this and reduce the number of functions. Instead pass a pointer to a variable to store the error into, or use errno? Would prefer you not test the addresses of out parameters for null. It should crash at this point, as this is a programming error, not a runtime error. Returning false will make it hard to figure out what is wrong. [IZ] can u please elaborate it bit more as I can add more test cases to cover the corner case if I understand clearly what do you want to highlight? I want it it CRASH if null is passed for the output pointer. The easiest way to do this is to not check if it is null. Please do not return zero or do anything other than crash right at that point. This is not an environment or input error, passing null for this is a programming error that can be fixed at compile time. Hiding the error so it is not obvious the first time the program is run is completely non-productive. It is useful to pass -1 for strtoul and this should continue to work. It is the only easily-recognized value for largest number possible. May be ok to not allow other negative numbers however. Yes I don't think preserving -1 is a big deal, but I have used this in the past. The caller can certainly check for this string before calling the converter. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On Tue, Nov 25, 2014 at 1:15 AM, Bill Spitzak spit...@gmail.com wrote: On 11/24/2014 11:32 AM, Imran Zaman wrote: [IZ2] This is the case where endptr is used in patch. I can remove endptr but it seems its used so i have to keep the existing functionality in place. + if (!weston_strtoi(pid, end, 0, other) || end != pid + 10) Use strlen(pid) != 10 instead. This will not exactly replace the implemented logic e.g. pid=123abcdefg. strlen(pid) = 10 end = 3 (end != pid + 10) So cant use the solution which u propose... [IZ2] Sorry still not able to grasp what you want to highlight. Can you please give an example? If I write the following code: if (strtol(string, 0, 0, 0)) ... I want it to crash so I notice that I passed 0 for the val output pointer. There is no reason to every pass null here so it must be a programmer mistake. As you have written it this will act like there is a syntax error in string, which could lead a person trying to debug their code down the wrong path. Sorry, why is it a programmer mistake and how would it crash? Let me add the exact test code and will come back to you. It should not crash for sure IMO. Also if the pointer is passed as (structure-member) it is going to crash anyway if structure is null and the offset to member is non-zero. I would think most errors causing the pointer to be invalid will be of this form so this is not preventing any crash. This cant be prevented anyway. we are using pointers everywhere in the whole weston code base. So its the user of the function who have to pass valid pointer or NULL. I suppose null could be used as I am only testing if it parses and don't want the number but it is not implemented this way, and I kind of doubt much code is interested in that test. At least one case above is using endptr.. if we can replace it with exactly similarly logic, I can remove it otherwies I dont see any harm in its usage ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
There is no need to pass endptr, because your functions will always fail
if it is not set to point to the terminating null.
I also suspect that the base is never used. In addition a possible
future fix would be to accept 0x for hex but a leading zero is decimal,
not octal, which means this function would generate the base.
I am also worried about passing the out ptr. Depending on the sizes of
int, long, and long long this will easily result in code that compiles
on one platform but not on another. Returning the value would avoid this
and reduce the number of functions. Instead pass a pointer to a variable
to store the error into, or use errno?
Would prefer you not test the addresses of out parameters for null. It
should crash at this point, as this is a programming error, not a
runtime error. Returning false will make it hard to figure out what is
wrong.
It is useful to pass -1 for strtoul and this should continue to work. It
is the only easily-recognized value for largest number possible. May
be ok to not allow other negative numbers however.
On 11/21/2014 07:38 AM, Imran Zaman wrote:
+static bool
+convert_strtol(const char *str, char **endptr, int base, long *val)
+{
+ char *end = NULL;
+ long v;
+ int prev_errno = errno;
+
+ if (!str || !val)
+ return false;
Please do not do the above tests!
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ errno = prev_errno;
+ *val = v;
+ return true;
+}
+
+static bool
+convert_strtoul (const char *str, char **endptr, int base, unsigned long *val)
+{
+ char *end = NULL;
+ unsigned long v;
+ int i = 0;
+ int prev_errno = errno;
+
+ if (!str || !val)
+ return false;
+ /* check for negative numbers */
+ while (str[i]) {
+ if (!isspace(str[i])) {
+ if (str[i] == '-')
+ return false;
+ else
+ break;
+ }
+ i++;
+ }
You could do the above test afterwards and check for and allow the -1 value.
+
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ errno = prev_errno;
+ *val = v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtoi(const char *str, char **endptr, int base, int *val)
+{
+ long v;
+
+ if (!convert_strtol(str, endptr, base, v) || v INT_MAX
+ || v INT_MIN)
+ return false;
+
+ *val = (int)v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtol(const char *str, char **endptr, int base, long *val)
+{
+ return convert_strtol(str, endptr, base, val);
+}
+
+WL_EXPORT bool
+weston_strtoui(const char *str, char **endptr, int base, unsigned int *val)
+{
+ unsigned long v;
+
+ if (!convert_strtoul(str, endptr, base, v) || v UINT_MAX)
+ return false;
+
+ *val = (unsigned int)v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtoul(const char *str, char **endptr, int base, unsigned long *val)
+{
+ return convert_strtoul(str, endptr, base, val);
+}
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On 21 November 2014 19:31, Bill Spitzak spit...@gmail.com wrote: It is useful to pass -1 for strtoul and this should continue to work. It is the only easily-recognized value for largest number possible. Other than ~0UL, ~0ULL, ULONG_MAX, etc ... ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On 11/21/2014 11:48 AM, Daniel Stone wrote: On 21 November 2014 19:31, Bill Spitzak spit...@gmail.com mailto:spit...@gmail.com wrote: It is useful to pass -1 for strtoul and this should continue to work. It is the only easily-recognized value for largest number possible. Other than ~0UL, ~0ULL, ULONG_MAX, etc ... That's a good point except strtoul does not parse those. It does parse -1. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
