svn commit: r283842 - head/usr.sbin/pw

[Bruce Evans]

On Mon, 1 Jun 2015, Baptiste Daroussin wrote:

> On Mon, Jun 01, 2015 at 03:36:58PM +1000, Bruce Evans wrote:
>> On Sun, 31 May 2015, Baptiste Daroussin wrote:
>>
>>> Log:
>>>  Remove useless cast in printf and printf-like functions:
>>>  use %u for uid_t and gid_t
>>
>> The cast was not useless.  It was to avoid the assumption that the default
>> promotion of uid_t and gid_t is anything in particular.  Now it is assumed
>> that the default promotion is unsigned (int works too, but this is subtler).
>>
>> uids and gids are only guaranteed to have non-negative values.  In
>> POSIX before about 2001, uid_t and gid_t can be any type that can
>> represent all the values taken on, so can be floating point.  Floating
>> point was rarely used for POSIX types, and most programs make too many
>> assumptions about types, so POSIX now requires uid_t and gid_t to be
>> integer types.  Then can still be signed integer types IIRC.  Nornally
>> it is a bug to print signed integer types with unsigned integer formats,
>> but since uids and gids are guaranteed to be non-negative both formats
>> work.  (pids require different handling since they are overloaded to
>> hold process group ids as negative values, so pid_t is signed and %u
>> format is very broken for printing general pids.)
>
> Well uid_t is defined as an unsigned int (__uint32_t actually) if it is ever
> going to be changed to something else they at least it will now raise a warning.

The definition is an implementation detail.  Knowing this detail defeats
the point of using typedefs.

> I consider using %u here is less buggy than the previous casts.

The casts were almost 100% correct (only had sign mismatches) when
written in ~1996 before C99 added uintmax_t and broke unsigned long.
Using %u don't even attempt to preserve the 1996 level of correctness/
carefulness.

pw is badly written so I'm surprised that it was this careful.  chpass
from 4.4BSD at least uses strtoul() to parse uids and gids, although
it has many of the usual errors in such use
   (first it blindly assigns the result of strtoul() to a uid_t.  Then
   it has a bogus check for ids being (uid_t)ULONG_MAX.  It is isn't
   clear if this is one of the usual errors in the use of strto*(),
   or just an obfuscated way of checking that the special id (uid_t)-1
   is not input...)
chpass int 4.4BSD was improved from chpass in Net/2 by switching from
atoi() to strtoul().

Note that unless you further switch all these strto*()s to strtoumax(),
ids larger than ULONG_MAX are just unsupported, so uid_t can't be
larger than uint32_t on 32-bit arches, and all the casts to u_long
remained 100% correct within this limit.

This shows that C99's breakage of long is much more than for printing.
All input methods that were careful enough to use strtol() or strtoul()
to parse values represented by typedefed integer types are now broken
unless they have been changed to use strtoimax or stroumax() (or more
usually, because the types are still no larger than long or u_long).
It will take a few more rounds of expansion before the larger types
are common.  On i386, u_long must remains 32 bits and use of 64 bits
for any basic type will cause much the same bugs as use of it for
off_t.  On amd64, u_long must remain 64 bits and it will take use of
__int128_t to break things.

Bruce