[freetds] new network timeout code

ZIGLIO, Frediano, VF-IT Frediano.Ziglio at vodafone.com
Tue Jan 9 03:51:30 EST 2007


> > > 
> > Is underscore part of libTDS function prefix?
> > Is underscore part of libTDS enumeration prefix?
> 
> Fair questions.  I'm thinking about it.  Certainly all the 
> existing libtds
> stuff has underscores.  Maybe it should be tds_perror().  
> 

I think that at this point someone should raise his hand and say "I
prefer tdserror" or "I prefer tds_error".

...
> 
> > - on timeout we call tdserror than later on caller (tds_goodread or
> > tds_goodwrite) we call tdserror again
> 
> I removed the outer loop in tds_select; it no longer calls tdserror.  
> 

Good.

> > - we can avoid to compute tv if ptv is NULL
> > - we should avoid to call tds_gettime_ms twice
> > - tv_usec unit is microseconds so milliseconds should be 
> multiplied by
> > 1000
> > - I don't understand tv.tv_sec + tv.tv_usec > 0 condition
> > 
> > Perhaps this would be easier
> 
> [excellent code snipped]
> 
> I tried a few different ways.  Yours was best.  Showoff.  ;-)
> 

I noted that recv/send return is not well tested. If these functions
return error (<0) we just assume we "unread" data and this could lead to
buffer underflow, memory corruptions and similar problems. Perhaps if
sufficient to remove the "else" after the first "if" in tds_goodread and
use same variable and similar way in tds_goodwrite.

...
> 
> > Perhaps however when someone will read documentation will 
> ask... What's
> > sock_errno? Perhaps should we specify errno on POSIX and
> > WSAGetLastError() on Windows? And under cygwin?
> 
> I think the code is easier to read as is.  It's a shame we can't just 
> 
> 	#define errno WSAGetLastError()
> 

I think that using a function to read/write current error is a good
thing (GetLastError/SetLastError in Windows) instead of macros and a
function call (errno is now something like "(*_get_errno_location())",
don't think as a "int errno") but I dislike Windows sockets cause they
are not integrated like Unix... you have to initialize and finalize them
(WSAStartup/WSACleanup), you use other functions to get/set errors,
SOCKETs are not real file handles (you can read from socket with
ReadFile but you can't use a file handle with select, nor read from file
with recv and many others differences).

> > Well... now I understand the recursion... a timeout inside
> > tds_send_cancel (however tds_send_cancel can't recurse). 
> I'm more aware
> > of a tds_send_cancel while sending a packet. Does this work 
> correctly?
> 
> I don't know yet.  I'll be sure to include it in my unit test.  
> 
> > Does even a tds_send_cancel inside a signal is correct?
> 
> If we're *very* careful.  The set of libc functions that are 
> signal safe
> is small.  
> 

This is another reason to use a simple variable. There is a test for
this in ctlib (cancel!).

freddy77



More information about the FreeTDS mailing list