FreeTDS Development Group

["ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>]

> 
> >>
> >> hi, I wrote the patch, so if you can give me more information
> >> on what
> >> revisions you are looking for, I'll take care of it.
> >>
> >> thanks,
> >> david
> >>
> >
> > Mainly it's just that I like more eyes looking at every patch before
> > apply. Also I'd like to see some comment for your questions
> > (client_charset use, CS_LC_ALL). I don't understand how you 
> handle dot
> > ('.') in language (CS_SYB_LANG_CHARSET). Are you sure that ctlib
> > locale->time is compatible with libTDS tds_socket->date_fmt ??
> 
> no problem at all.
> 
> Regarding CS_SYB_LANG_CHARSET: I implemented the support for this  
> following Sybase' OpenClient 15 manual as documented here:
> http://infocenter.sybase.com/help/index.jsp?topic=/ 
> com.sybase.dc32850_1500/html/comlib/X37122.htm
> 
> I then tested the code against real Sybase ct-lib and against my  
> patched freetds version and it worked the same (checked character  
> encodings of queries and the server messages for the language 
> set).    
> I also tested error cases (unknown languages, etc) and they 
> worked as  
> expected with both Sybase's ct-lib and my patched freetds code.
> 

Do you still have these tests??
It would be nice to have them as unit tests. I think this is a nice way
to have a consistent behavior in future release.

> I do not believe that locale->time is compatible with tds_socket- 
>  >date_fmt.  However the code to set this value is not included in  
> the patch - I assumed that in the future, when some translation can  
> be made between languages and time formats somehow, that an  
> appropriate string for tds_socket->date_fmt could be generated from  
> the CS_LC_TIME value, but maybe this is not enough.
> 

I asked this cause in ct_connect you copy locale->time to
tds_socket->date_fmt.

> I also ran tests with valgrind to check for memory leaks, memory  
> errors (found and fixed a couple) and the result code in the patch  
> was memory-clean.
> 

Yes, it happear correct even to me, there is only a small leak when
connection fails (fixed).

> However I see now that there is potential a memory bug there in the  
> patch (which didn't get caught because this code path can never be  
> reached at the moment for the reasons described above) - in ct.c  
> ct_connect(): strlen(con->locale->time) - 1 should be + 1 instead.
> 

Even better to use strdup. 

> thanks,
> David
> 
> PS: the confusion over what you wanted with the patch was due to  
> "false friends" between english and italian: revisitare = "review",  
> "revise" = editare, rifare
> 

ok... I'll try to remember :)

freddy77