Re: Casting ctype lookups

[David Holland <dholland-current%netbsd.org@localhost>]

On Wed, Nov 14, 2012 at 07:51:23AM -0500, D'Arcy J.M. Cain wrote:
 > On Mon, 12 Nov 2012 13:47:59 -0500
 > "D'Arcy J.M. Cain" <darcy%NetBSD.org@localhost> wrote:
 > > Should the inline macro cast the args for these table lookups?  I
 > > would like to modify sys/ctype_inline.h to change lookup lines like;
 > > 
 > > #define toupper(c)  ((int)((_toupper_tab_ + 1)[(c)]))     
 > > to
 > > #define toupper(c)  ((int)((_toupper_tab_ + 1)[(int)(c)]))
 > > 
 > > or else add a comment why they should not be cast.  Which should it
 > > be?
 > 
 > Would this be safer?
 > 
 > #define toupper(c)\
 >   ((int)((_toupper_tab_ + 1)[(int)(unsigned char)(c)]))

As already pointed out, that would be wrong.

The reason the warning is somewhat obscure is that it is a warning
about an only tangentially related condition that someone figured out
how to leverage into a warning about incorrect <ctype.h> usage.

The gcc warning warns about "pointer[c]" where c has type char,
because the resulting program behavior is implementation-defined:
depending on whether the unqualified char type is signed or unsigned
the pointer will be indexed differently if the value is not in the
range 0..SCHAR_MAX. While it is conceivably possible that some program
might take advantage of this behavior on purpose, in general it
represents a mistake, so gcc warns about it.

Meanwhile, the definition of isupper() and toupper() and the rest is
only valid for argument values ranging from -1 (EOF) to UCHAR_MAX. A
standard mistake is to pass values of type char; on platforms where
the unqualified char type is signed, which is most of them, values
outside the 0..SCHAR_MAX range are sign-extended to negative int
values and produce undefined behavior.

NetBSD's <ctype.h> has been intentionally rigged to produce a warning
for this case, via the mechanism explained above. Please don't try to
"fix" it. As code that triggers this warning is incorrect, please fix
the offending code instead.

If you can come up with an alternate mechanism to produce a more
cogent warning, that would be great, but it probably requires patching
gcc.

-- 
David A. Holland
dholland%netbsd.org@localhost