Skip to Content.
Sympa Menu

freetds - Re: [freetds] new network timeout code

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: "ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>
  • To: "FreeTDS Development Group" <freetds AT lists.ibiblio.org>
  • Subject: Re: [freetds] new network timeout code
  • Date: Tue, 9 Jan 2007 09:51:30 +0100

> > >
> > 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





Archive powered by MHonArc 2.6.24.

Top of Page