FreeTDS Development Group

[David Nichols <nicholsman AT gmail.com>]

On Jun 21, 2007, at 11:48 AM, ZIGLIO, Frediano, VF-IT wrote:

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

unfortunately I did not do it in a way that the tests could be kept.   
I did the tests as I was developing the freetds driver for qore - the  
same source is compiled with sybase' ct-lib and freetds and I checked  
the results in each in a cycle where I would modify the code, compile  
both drivers, review results, etc.  For future patches, and when I  
have time to come work on this again, I will create unit tests in the  
unittest directory for all changes...

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

Yeah, I was hoping to be able to implement this functionality as  
well, but it looked too time-consuming, and I didn't need it at the  
time..

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

oops!

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

yes absolutely right.

thanks
david

> _______________________________________________
> FreeTDS mailing list
> FreeTDS AT lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds