On Tue, 16 Jan 2018 21:10:36 +0200
Vladislav Valtchev <vladislav.valtc...@gmail.com> wrote:
> On Tue, 2018-01-16 at 12:19 -0500, Steven Rostedt wrote:
> > On Tue, 16 Jan 2018 09:47:42 +0200
> > "Vladislav Valtchev (VMware)" <vladislav.valtc...@gmail.com> 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].
>
True. What about just doing:
if (num > INT_MAX || num < INT_MIN || (!num && errno))
That should cover it all, and match what the man pages have.
-- Steve
