Re: [PATCH] Catch under/overflow cases in cvtnum() and cvttime().

Arkadiusz Miśkiewicz
2014-07-12T06:13:35Z

On Saturday 12 of July 2014, Eric Sandeen wrote:
> On 7/11/14, 2:34 PM, Arkadiusz Miśkiewicz wrote:
> > cvtnum() and cvttime() silently ignore overflows. This leads to error
> > conditions not being catched. Example:
> >
> > $ xfs_quota -x -c 'limit -u bsoft=987654321098765432199 \
> >
> > bhard=987654321098765432199 999' /
> >
> > $
> >
> > Fixed version:
> > $ xfs_quota -x -c 'limit -u bsoft=987654321098765432199 \
> >
> > bhard=987654321098765432199 999' /
> >
> > xfs_quota: Error: could not parse size 987654321098765432199.
> > xfs_quota: unrecognised argument bsoft=987654321098765432199
>
> So, strtol(3) suggests setting errno to 0 before the call:
>
> NOTES
> Since strtol() can legitimately return 0, LONG_MAX, or
> LONG_MIN (LLONG_MAX or LLONG_MIN for strtoll()) on both success and
> failure, the calling program should set errno to 0 before the call, and
> then deter- mine if an error occurred by checking whether errno has a
> non-zero value after the call.
>
> Ditto for strtoul().

Hm, my man pages 3.70 don't have such notes, strtol(3):

NOTES
In locales other than the "C" locale, also other strings may be
accepted. (For example, the thousands separator of the current locale may be
supported.)

BSD also has

quad_t
strtoq(const char *nptr, char **endptr, int base);

with completely analogous definition. Depending on the wordsize of the
current architecture, this may be equivalent to strtoll() or to strtol().

>
> I guess that is just to ensure that there's not a leftover errno
> when we make the call? Worth doing, maybe?

ERANGE is checked in few other places already in input.c and none initialize
errno before strtoul() call.

>
> Thanks,
> -Eric
>
> > Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
> > ---
> >
> > libxcmd/input.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/libxcmd/input.c b/libxcmd/input.c
> > index c06b5b8..397a124 100644
> > --- a/libxcmd/input.c
> > +++ b/libxcmd/input.c
> > @@ -154,6 +154,8 @@ cvtnum(
> >
> > int c;
> >
> > i = strtoll(s, &sp, 0);
> >
> > + if ((i == LLONG_MIN || i == LLONG_MAX) && errno == ERANGE)
> > + return -1LL;
> >
> > if (i == 0 && sp == s)
> >
> > return -1LL;
> >
> > if (*sp == '\0')
> >
> > @@ -238,6 +240,8 @@ cvttime(
> >
> > char *sp;
> >
> > i = strtoul(s, &sp, 0);
> >
> > + if (i == ULONG_MAX && errno == ERANGE)
> > + return 0;
> >
> > if (i == 0 && sp == s)
> >
> > return 0;
> >
> > if (*sp == '\0')

--
Arkadiusz Miśkiewicz, arekm / maven.pl