FreeTDS Development Group

[Frediano Ziglio <freddy77 AT gmail.com>]

Il giorno lun, 11/01/2010 alle 10.21 -0500, jklowden AT schemamania.org ha
scritto:
> 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.) 
> 

It's also true that oserr is used only by dblib and passed to client
application on error handler... Perhaps would be better to translate C
errors into Windows system errors and use FormatMessage for windows and
strerror for Unix?

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

Yes, but can be not thread-safe on some Unix... we always use strerror,
windows or not.

> > 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 just did some cleanup after compiler told me that some functions was
unused...

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

Catched... but I think that currently does not work.

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

In recent version there is no static in types.h so data.c "export"
tds_get_varint_size. I think you should update. I think that a developer
have to use perl even on windows or manually copy file from distribution
while users will use distribution which came with proper file.
Why not adding .sln/.vcproj and Nmakefile to distribution ??

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

It's that macro is defined in replacements.h and tds_sysdep_private.h so
I got a warning about redefinition. I build defncopy as cross compile
using FreeTDS library (not MS one). Yes, MingW define str(n)casecmp in
string.h so I have no problem, probably is better to not define
str(n)casecmp if MingW is detected I think mostly windows compilers do
not define these functions. It seems also that Visual Studio 2005 give
warning suggesting the use of _stricmp instead of stricmp. 
Perhaps defncopy.c should include tds_sysdep_private instead ?? This
define also strdup and other macro very useful.

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

Committed.

> --jkl
> 

freddy77