FreeTDS Development Group

[jklowden AT schemamania.org]

On Sun, Jan 10, 2010 at 02:27:53PM +0100, Frediano Ziglio wrote:
> 2010/1/9 James K. Lowden <jklowden AT freetds.org>:
> > tdserror() assumes errnum can be converted to a string with strerror(3).
> > On Windows that's not true; for winsock errors strerror(3) returns
> > "Unknown error". ?Somehow, we need to pass the Win32 error string
> > (returned from tds_prwsaerror()) to the client libraries. ?But how?
> 
> The problem is that in Windows C errors (from errno) and system errors
> (from GetLastError/WSAGetLastError or returned from APIs depending on
> API set!) 

Yes. 

> > My suggestion: add
> >
> > ? ? ? ?char * winsock_errstr
> 
> I don't like this... perhaps would be better to understand if is a
> socket error from tds error code.

OK.  winsock_errstr is a flag, too.  If set, it's a winsock error, or other 
error for which strerror(3) is no help.  

Maybe we should just call it "errstr".  It would be set whenever strerror() 
is no use.  dbperror() would say something like:

        fprintf( "...%s...", tds->errstr? tds->errstr : strerror() )

> Looking at windows constants I would use a trick and an unique error
> number. C errors in windows are very few (the higher constant is 80
> but is not used by FreeTDS) while socket errors are usually higher
> than 10000. 

The advantage to your idea is that function that provoked the error doesn't 
need to do anything to tell the error-printing logic which API created the 
error.  And it's threadsafe.  It's also fragile, and unobvious to anyone 
reading the code. 

Instead of depending on the values, we could see if strerror(3) succeeds in 
dbperror():

        errno_copy = errno;
        errno = 0;
        msg = strerror(errno_copy);
        if( errno != 0 )
                msg = tds_prwsaerror(); 
        fprintf( "...", msg );
        

(I don't know if Microsoft's strerror(3) reliably sets errno if it fails.) 

> Perhaps we could define a tds_strerror that take this
> constants and output a string. (see
> http://msdn.microsoft.com/en-us/library/t3ayayh1%28VS.80%29.aspx and
> http://msdn.microsoft.com/en-us/library/ms740668%28VS.85%29.aspx).

Cf. net.c::tds_prwsaerror().  

Win32 strerror(3) uses _sys_errlist and _sys_nerr.  If strerror(3) doesn't 
work (i.e. winsock) then _sys_errlist and _sys_nerr won't work either.  

> Another problem is thread safety. strerror is not thread safe, it
> would be better to use strerror_r if available. Under Windows for C
> errors there is _sys_nerrs and _sys_errlist.

Not exactly: 

http://www.opengroup.org/onlinepubs/000095399/functions/strerror.html

        "The strerror() function need not be reentrant."
        
Need not be, but may be.  I think Win32 strerror(3) is threadsafe. 

I don't have the source code for Microsoft's strerror(3), but the header 
files define errno, _sys_nerrs, and _sys_errlist as macros that call 
functions that *should* return thread-local values.  

> Good! I saw you committed patch for defncopy and Windows

Oops!  I didn't mean to commit it.  It's a bit of a mess.  It was just a 
quick hack so I could use it.  But it's much better now, thanks!  

> I saw some strange include in defncopy like
> replacements.win32.hacked.h, win32.microsoft directory. 

The idea is to let it compile with FreeTDS or Microsoft's library.  If we 
want to support that generally, we need a way to supplement Microsoft's libc 
e.g. getopt(3).  

> Also there is
> a strange types.h inclusion in src/tds/token.c... quite strange,
> tds_get_varint_size should be defined in data.c (which include
> types.h), this breaks my cross Windows compile. 

On my system I have E:\freetds\build\src\tds\types.h generated by types.pl 
except for Win32/64.  I also have include/x64/types.h (which is used in the 
win32 build, too) that is based types.pl,v 1.1 2008/09/17 12:16:09, which 
included:

/**
 * tds_get_varint_size() returns the size of a variable length integer
 * returned in a TDS 7.0 result string
 */
#if !ENABLE_EXTRA_CHECKS
static
#endif
int
tds_get_varint_size(TDSSOCKET * tds, int datatype)

I see now ENABLE_EXTRA_CHECKS is not part of the current types.h.  

What file provides tds_get_varint_size in your Win32 build?  (Let me guess.  
You build Win32 from the tarball, which includes types.h.  I'm building from 
CVS, so I *should* run Perl myself to generate it.  OK.  More work for 
Nmakefile.) 

> Also I got a double
> definition cause you added a strncasecmp in include/replacements.h.
> Perhaps should be placed in defncopy.c (like strcasecmp define?).

When you build Win32 as cross-compile, strncasecmp(3) is a function provided 
by the libc?  <Maybe we need a way to distinguish between using Microsoft's C 
runtime and others.  Do you know of a symbol defined only by Microsoft's 
header files, so we could do:

        #if USING_MICROSOFTS_H_FILES
        # define strncasecmp(x,y,z) stricmp((x),(y),(z))
        #endif

> Did you see my initial patch for forbid instance and port
> specification on freetds.conf?? I attach it.

I don't completely understand it, but please commit it and we'll try it out.  

--jkl