FreeTDS Development Group

["Lowden, James K" <LowdenJK AT bernstein.com>]

> From: Christos Zoulas
> Sent: Thursday, March 10, 2005 8:53 AM
> 
> On Mar 10,  1:18pm, Frediano.Ziglio AT vodafone.com wrote:
> -- Subject: RE: [freetds] casts and warnings
> 
> | Happily gcc under HP-UX give me this warning. Perhaps a better way
is to
> | define a macro like TDS_ISSPACE (already defined in some C file) to
> | handle this case. This can cause a buffer reading underflow with
> | undefined behavior (core or bad results). If I remember under Linux
the
> | "array" is defined from -128 to 255 to handle signed chars
correctly.
> 
> You are going to have to do this for all the is*() macros, but yes,
that
> would save you typing a few characters each time you use it.

Christos, 

In an earlier message, you made the irrefutable point (and, for me,
that's saying something) that because some charsets include 8-bit
characters, treating a character as a signed entity is a source of
trouble whenever it's promoted to int.  I spent a few hours researching
that last night.  It's not easy to find definitive sources of model
code.  "The Annotated Annotated C Standard"
(http://www.lysator.liu.se/c/schildt.html) points out that even the
Standard itself includes error-prone illustrations, cf. 7.9.10.2 and
7.9.10.3.  

My conclusion differs from everyone's, including my previous one.  ;-)
The real error, it seems to me, and something I'm going to peruse the
code for, is treating *any* character data as signed.  This loop:

        char query_line[4096];
         ...
        while (fgets(query_line, sizeof(query_line), stdin)) {
                const char *p = query_line;
                if (strncasecmp(p, "go", 2) == 0) {
                        for (p+=2; isspace((unsigned char) *p); p++) {
                                if (*p == '\n')
                                        return 1;
                        }
                }

should be

        unsigned char query_line[4096];
         ...
        while (fgets((char*) query_line, sizeof(query_line), stdin)) {
                const unsigned char *p = query_line;
                if (strncasecmp((char*) p, "go", 2) == 0) {
                        for (p+=2; isspace(*p); p++) {
                                if (*p == '\n')
                                        return 1;
                        }
                }

Why?  

The right approach -- fighting words, to be sure -- is to declare the
data accurately.  fgets(3) really does return an array of 8-bit
entities; its char* argument is an ASCII anachronism (although "plain
char" may be signed or not).  In no sense do the elements of query_line
ever represent negative values.  

Once we know we're dealing with an unsigned character, we can allow it
to be promoted to  int willy-nilly.  Granted, we have to be careful of
contructions like (*p - '0') that might wrap around, but that's just a
variation of the usual bounds checking.  

I think declaring the data unsigned will lead to fewer casts and (by
definition) cleaner code, don't you?  

More important, most compilers will complain about passing unsigned
char* to a function takikng char*, whereas many will not (and should
not) complain about passing a char to an int when there is an implicit
unsigned constraint.  IOW, I'm sure there are other cases of calling
isspace & Co. with a plain char.  Declaring the input unsigned (where
appropriate) fixes all of them and all future ones, at the cost of
requiring casts for char*.  

Of course, of course, we're only postponing the real problem.  None of
this works with UTF-8.  

My thanks to you and the others for the education.  

--jkl

-----------------------------------------
The information contained in this transmission may contain privileged and 
confidential information and is intended only for the use of the person(s) 
named above. If you are not the intended recipient, or an employee or agent 
responsible for delivering this message to the intended recipient, any 
review, dissemination, distribution or duplication of this communication is 
strictly prohibited. If you are not the intended recipient, please contact 
the sender immediately by reply e-mail and destroy all copies of the original 
message. Please note that we do not accept account orders and/or instructions 
by e-mail, and therefore will not be responsible for carrying out such orders 
and/or instructions.
If you, as the intended recipient of this message, the purpose of which is to 
inform and update our clients, prospects and consultants of developments 
relating to our services and products, would not like to receive further 
e-mail correspondence from the sender, please "reply" to the sender 
indicating your wishes.  In the U.S.: 1345 Avenue of the Americas, New York, 
NY 10105.