Subject: Re: isspace and compiler warnings
To: None <>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: tech-userlevel
Date: 10/18/2006 11:13:28
> ufraw_routines.c:262: warning: array subscript has type 'char'
> ufraw_routines.c:263: warning: array subscript has type 'char'

>       for(; len>0 && isspace(*text); len--, text++);
>       for(; len>0 && isspace(text[len-1]); len--);

I've had run-ins with that myself.

> man isspace says that it is a function taking an int.
> But, our implementation is a macro, so promotion doesn't happen and
> the warning occurs due to the expansion.

As it happens, this is good, because you get warned about broken code;
if it were a real function the argument would get promoted silently and
the code would still be broken.

The is*() macros/functions take an unsigned char value or EOF, the
latter being why they formally take int.  (The API design is slightly
broken in that it cannot work right on architectures where char and int
are the same, but that's not a problem for NetBSD.)  Code that passes a
signed char (whether plain char or explcitly signed) to them is broken.

Thus, code that passes plain char is broken (unless it is
architecture-specific code not used for arches with signed chars).  As
martin@ says, in this case the argument cannot be EOF (since it comes
from a string, not (eg) getchar), so a cast to unsigned char is the
appropriate treatment.  Whether you apply the cast after following the
pointer or before is a matter of taste.  (That is, whether you write
	isspace((unsigned char)*text)
	isspace(*(unsigned char *)text)
or, equivalently to the latter, create an unsigned char * variable for
the purpose.)

I do note that text is declared as "const gchar *".  What's a gchar?
If it can be wider than a char, the code is broken even worse on all
arches where that can be the case.

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B