FreeTDS Development Group

[entropy AT freetds.org]

Lowden, James K wrote:
> Frediano recently made a small change to bsqldb.c to address a warning.
> I looked at the change, and I think should treat warnings and casts a
> little more carefully.  Here's the change:
>
> @@ -228,7 +228,7 @@
>                 /* 'go' or 'GO' separates command batches */
>                 const char *p = query_line;
>                 if (strncasecmp(p, "go", 2) == 0) {
> -                       for (p+=2; isspace(*p); p++) {
> +                       for (p+=2; isspace((unsigned char) *p); p++) {
>
> And the standards:
>
> http://www.opengroup.org/onlinepubs/007908799/xsh/isspace.html
> http://www.vmunix.com/~gabor/c/draft.html#6.2.1.1
>
> isspace(3) takes an int; we passed it a signed char.  ANSI says you can
> use  a signed char wherever an int is called for.  So, isspace(*p) is on
> standard turf.  
>
> Moreover, the whole point of isspace(3) is to test a char -- almost
> always a signed char, because ASCII defines 7-bit characters.  The code
> as written was completely reasonable, normal, standard C idiom.

Wrong.  The code is correct with the cast and incorrect without it.  The 
calling code needs to ensure that when *p is sign extended it will be a 
positive integer.  ASCII may define 7-bit characters but char is an 
8-bit datatype and can be negative on systems with signed chars.

Quoting the SUS that you cited, emphasis added:

In all cases c is an int, the value of which must be a character 
***representable as an unsigned char*** or must equal the value of the 
macro EOF. If the argument has any other value, the behaviour is undefined.

Clearly a negative signed char is not representable as an unsigned char, 
so we need to ensure that we never pass such a value to isspace().

Cheers,
entropy