Re: A couple of bugs in 0.91 with dblib bcp library

Il giorno sab, 21/05/2011 alle 14.32 -0400, James K. Lowden ha scritto:
> On Sat, 21 May 2011 08:35:29 +0100
> "Thompson, William" <[email protected]> wrote:
> 
> Hi Bill, 
> 
> > -        assert(bufpos > 0);
> > +        assert(bufpos >= 0);
> 
> Committed.  
> 

I did commit just some hour before :)
You add a relaxation to term_len, not to bufpos.

> > -         CHECK_PARAMETER(dbproc->bcpinfo, SYBEBCPI, FAIL);
> > +         if (!(dbproc->bcpinfo))
> > +                return 0;
> 
> Committed, except IMO the return code should be -1.  My reasoning is
> the application should be able to detect a logical error, and the
> function is misused if called twice without an intervening (and
> successful) bcp_init().  
> 
> If the application is cavalier:
> 
> 	bcp_done(dbproc);
> 	bcp_done(dbproc);
> 
> will now succeed and fail silently, in that order.  If it's careful:
> 
> 	if( bcp_done(dbproc) == -1 ) {
> 		/* do error stuff */
> 	}
> 	if( bcp_done(dbproc) == -1 ) {
> 		/* do error stuff */
> 	}
> 
> it will break, as well it should, as the programmer should prefer.  
> 

I think that the idea behind Bill relaxation is really return: I have
inserted 0 row (that is 0 return), not returning an error.
For some reason Bill code issue an additional bcp_done without rows and
Sybase code used to return 0 while FreeTDS returns error.

Have you noted that this function is returning FAIL (0) instead of -1?
How I miss C++ and its type safety, I would use an enum and compiler
gave me at least a warning :)

> If that doesn't suit you, I'll #ifdef it for you.  
> 

#ifdef FIX_BILL_PROBLEMS ?? 

bye
  Frediano