[freetds] casts and warnings
entropy at freetds.org
entropy at freetds.org
Wed Mar 9 16:15:44 EST 2005
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
More information about the FreeTDS
mailing list