FreeTDS Development Group

["James K. Lowden" <jklowden AT freetds.org>]

Frediano Ziglio wrote:
>        if (host_term == NULL && host_termlen > 0 
>         || host_term != NULL && host_termlen == 0) {
>                 dbperror(dbproc, SYBEVDPT, 0);  /* "all
> variable-length data must have either a length-prefix ..." */
>                 return FAIL;
>         }
> 
> I removed a similar warning in dblib.c, see
> http://freetds.cvs.sourceforge.net/viewvc/freetds/freetds/src/dblib/dblib.c?r1=1.343&r2=1.344&sortby=date
> which IMHO is more readable.

Indeed!  

> Probably code should be
> 
>         if (host_term == NULL && host_termlen > 0 
>          || host_term != NULL && host_termlen <= 0) {

That looks better.  I wonder, does 

        host_term == NULL && host_termlen > 0

matter?  Sybase says, "To avoid using a terminator, set this parameter to
NULL."  What harm if the user sets host_termlen to something other than 0
or -1?  

It's a choice between protecting the library and protecting the
programmer.  On one hand, if either host_term is NULL or host_termlen is
[0,-1], there's no terminator.  On the other hand, if the programmer
provides inconsistent information, maybe he's confused and should get an
error.  I don't know what the right answer is.  

Regarding the nanny worrywart gcc recommendation about parenthesis, I
guess you know my opinion.  One compiler issues that warning if certain
options are used.  I don't see that as a reason to litter the code with
parentheses in expressions that are perfectly clear without them.  

I wrote to gcc about it, and I know they agree that finer-tuned warnings
would be an improvement.  I hope they'll remove this one from -Wall, but
I'm not sure I won that argument.  

Regards, 

--jkl