Subject: Re: disklabeling a 1.7 TB disk
To: None <firstname.lastname@example.org>
From: David Laight <email@example.com>
Date: 02/28/2004 23:19:31
On Sat, Feb 28, 2004 at 11:59:23PM +0100, Christian Biere wrote:
> Jun-ichiro itojun Hagino wrote:
> > more error check to strtoul().
Not really worth the effort.....
> Eh, is invalid input unimportant here? Sorry, I really don't see why
> anybody would use atoi(), atol() etc. unless you already know that
> the given parameter is a valid value within the correct range. These
> functions are *crap*, period.
True - but they are moderately ok when the input is controlled.
In this case the only read problem is that atoi() saturates at 2^31-1.
> > + u_int32_t m; \
> Wouldn't it be better to use uintmax_t here?
The values are being written into fields defined as u_int32_t....
> > \
> > _CHECKLINE \
> > cp = tp, tp = word(cp); \
> > - m = (int)strtol(cp, &ptr, 10); \
> > - if (*ptr == '\0') \
> > + errno = 0; \
> > + m = strtoul(cp, &ptr, 10); \
> > + if (*cp && ptr && *ptr == '\0' && errno == 0) \
> unsigned long isn't necessarily an alias for u_int32_t and strtoul() happily
> accepts negative values.
true, but does it matter?
Fixing this program properly would require daddr_t in the disklabel.
> Hmm, if it's save to assume (errno == 0) in case of a successful strtoul()
> you might want to shorten the other checks to "errno" instead of
> "errno == ERANGE". Otherwise, I'd change this to "errno != ERANGE".
Or if you are willing to say '*cp && ptr', why not '!*ptr && !errno' ?
In any case the point of teh fix was to allow numbers between 2^31 and
2^32-1 to be input, not to fix the error checking.
David Laight: firstname.lastname@example.org