bin/142911: [patch] vmstat(8) -w should produce error message if fed a negative value

[Bruce Evans]

The following reply was made to PR bin/142911; it has been noted by GNATS.

From: Bruce Evans <brde at optusnet.com.au>
To: Efstratios Karatzas <gpf.kira at gmail.com>
Cc: bug-followup at FreeBSD.org, Bruce Evans <brde at optusnet.com.au>
Subject: Re: bin/142911: [patch] vmstat(8) -w should produce error message
 if  fed a negative value
Date: Wed, 20 Jan 2010 01:04:37 +1100 (EST)

 On Mon, 18 Jan 2010, Efstratios Karatzas wrote:
 
 > I still can't understand why would anyone use "vmstat 0" or "vmstat -w 0".
 > It's the same output if you just use "vmstat".
 > I am aware of the BACKWARD_COMPATIBILITY behavior but negative values
 > don't work with it, for example "vmstat -3" produces an illegal option error.
 
 That accidentally avoids the int -> unsigned conversion error.
 
 However, there is another conversion here, not fixed by your version:
 strtol() returns long, so on 64-bit arches, since long is actually long,
 it is longer than int, so when atoi blindly converts the value to int
 it gets truncated to a garbage value if the correct value is > INT_MAX.
 E.g.:
 
  	2**31 becomes -2**31 (accidentally handled as error, with confusing
  			      error message about the arg being < 1)
  	2**32 becomes 0      (accidentally...)
  	2**32+1 becomes 1    (error not detected)
  	2**63 first becomes LONG_MAX (2**63-1) with error ERANGE in strtol()
  	      becomes 2**31-1 (error not detected)
          > 2**63: same as 2**63.
  	-2**31 stays as -2**31
  	-2**32 becomes 0
  	-2**32+1 becomes 1
  	>= 2**-63 first becomes LONG_MIN, then INT_MIN (error detected).
 
 The large values are not useful so they shouldn't be used, but the same is
 true for negative values.
 
 > Although "vmstat abc" will work just fine but it is not much of a problem.
 >
 > We could perform a more "sophisticated" error check.
 > Elaboration:
 >
 > A call to atoi(3) is actually a call to strtol(3).
 > strtol(3) will return 0 and set errno to EINVAL if no conversion could be
 > performed.
 > eg "vmstat -w abc"
 > So we could check the value of errno when atoi() returns 0.
 
 After setting errno to != EINVAL before the call.
 
 > But the man page states that this feature is not portable across all platforms!
 
 This is not even portable to POSIX systems.  On POSIX systems, the POSIX is
 also undefined on overflow.  Other errors are only implementation-defined.
 This depends on the behaviour being the same as casting strtol() to int,
 as documented, and on strtol() having a new misfeature in POSIX (that
 errno is clobbered to EINVAL for some but not all types of parsing
 errors).
 
 This behaviour should never be depended on.  Just use strtol() with
 normal error checking (check that the end pointer has advanced and
 doesn't point to garbage).  errno doesn't need to be checked for this.
 However, errno needs to be set to != ERANGE before the call and checked
 == ERANGE after the call to check for range errors.  Then the result
 must be checked to fit in the int or unsigned variable.
 
 FreeBSD has 2 nonstandard badly designed interfaces that are intended
 to be easier to use than strto*() and atoi(): strtonum() and
 expand_number().  Their bad design starts with then not supporting
 intmax_t, uintmax_t or any floating point type.  strtonum() doesn't
 even support unsigned longs on 64-bit arches, and it uses the long
 long abomination.  But it is an adequate replacement for strtol(),
 and thus OK for replacing atoi() in most command option parsing.
 
 Another issue is whether numeric command options should be limited to
 decimal values representable as a long or even as an int.  I think they
 shouldn't have any arbitrary limits, but POSIX may over-specify them as
 being decimal and/or of limited size.  Another design bug in strtonum()
 is that it only supports decimal numbers.  This makes it more compatible
 with atoi() but less useful than it should be.
 
 Bruce