On Tue, 2018-01-16 at 12:19 -0500, Steven Rostedt wrote:
> On Tue, 16 Jan 2018 09:47:42 +0200
> "Vladislav Valtchev (VMware)" <[email protected]> wrote:
>
> > + errno = 0;
> > +
> > + /* Read an integer from buf ignoring any non-digit trailing characters.
> > */
> > + num = strtol(buf, NULL, 10);
> > +
> > + /* strtol() returned 0: we have to check for errors */
> > + if (!num && (errno == EINVAL || errno == ERANGE))
> > + return -1;
>
> Repeating again here. According to the man page of strtol():
v3 addresses only the comments for patch 3/3.
I'm sorry for that. All the other comments will be addressed in v4.
>
> RETURN VALUE
> The strtol() function returns the result of the conversion, unless the
> value would underflow or overflow. If an underflow occurs, strtol()
> returns LONG_MIN. If an overflow occurs, strtol() returns LONG_MAX.
> In both cases, errno is set to ERANGE. Precisely the same holds for
> strtoll() (with LLONG_MIN and LLONG_MAX instead of LONG_MIN and
> LONG_MAX).
>
> and this:
>
> The implementation may also set errno to EINVAL in case no conversion
> was performed (no digits seen, and 0 returned).
>
> Thus, !num is not enough. The example in the man page has:
>
> errno = 0; /* To distinguish success/failure after call */
> val = strtol(str, &endptr, base);
>
> /* Check for various possible errors */
>
> if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
> || (errno != 0 && val == 0)) {
> perror("strtol");
> exit(EXIT_FAILURE);
> }
>
> Let's follow this.
>
> -- Steve
Sure, I thought that:
errno = 0;
num = strtol(buf, NULL, 10);
/* strtol() returned 0: we have to check for errors */
if (!num && (errno == EINVAL || errno == ERANGE))
return -1;
if (num > INT_MAX || num < INT_MIN)
return -1;
covered all the cases because the case:
(val == LONG_MAX || val == LONG_MIN)
is covered by: if (num > INT_MAX || num < INT_MIN)
[no matter the errno]
but that's not true for 32 bit systems where sizeof(long) == sizeof(int).
It had to be: if (num >= INT_MAX || num <= INT_MIN), but in that
case it would exclude two valid int32 values.
Therefore, let's go with:
if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
|| (errno != 0 && val == 0))
Just let me keep also the following check:
if (num > INT_MAX || num < INT_MIN)
return -1;
since [INT_MIN, INT_MAX] is a subset of [LONG_MIN, LONG_MAX].
Vlad
--
Vladislav Valtchev
VMware Open Source Technology Center
