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

[Eric Sandeen]

On 7/12/14, 8:37 AM, Eric Sandeen wrote:
> On 7/12/14, 1:13 AM, Arkadiusz Miśkiewicz wrote:
>> 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.
> 
> http://c-faq.com/misc/errno.html suggests it too:
> 
>> It's only necessary to detect errors with errno when a function does
>> not have a unique, unambiguous, out-of-band error return (i.e.
>> because all of its possible return values are valid; one example is
>> atoi). In these cases (and in these cases only; check the
>> documentation to be sure whether a function allows this), you can
>> detect errors by setting errno to 0, calling the function, then
>> testing errno.
> 
> I wonder why it was removed from the man page

Actually it seems to still be there:
http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man3/strtol.3#n190

fiddly detail but probably worth doing...

-Eric