FreeTDS Development Group

[ZIGLIO Frediano <Frediano.Ziglio AT vodafoneomnitel.it>]

> 
> Hi Freddy,
> 

Hi Bill

> I'm sorry you're disappointed with my patch.
> However, I think it's a little harsh to say that we are "back to the
> drawing board". Let me go through your points, and try and 
> answer them...
> 

Perhaps not to drawing but version 0.6 is not a "next week" release.

> > I see that many work are shifted from tds to dblib/ctlib 
> library. But many
> > conversion can be done in tds_convert (such as same type or other).
> > 
> 
> The work that has shifted from tds_convert to dblib/ctlib is:
> 
> a) like-to-like conversions
> b) post conversion assignment of data back to the calling program
> 
> I moved like-to-like conversions out of tds_convert, because 
> I saw it as
> unnecessary to go into the conversion code unless strictly 
> necessary. At
> the end of the day, It's not integral to the architecture of 
> my patch to
> do this - tds_convert() will still handle like-to-like conversions.
> 

Yes, like-to-like is now a cut and paste between ctlib and dblib
 
> > I think that we must pass to tds_convert pointer to final 
> char buffer and
> > destination length (as before). tds_convert should fill 
> destination buffer
> > until filling them. Library should then zero-terminate or 
> padding (as
> > necessary) or return error. This solve the double copy, 
> unnecessary memory
> > allocation and library problems. Some can disappoint that 
> dblib/ctlib can be
> > called with destlen -1 or -2 (there is sufficient space). 
> This is not a
> > problem, just fill destlen with maximun allowed (2GB in x86 
> architecture).
> 
> We had long discussions about this before. My view was firmly that ALL
> responsibility for touching the calling programs variables should be
> devolved to the API layer. This was because each API has different
> standards about how this should be done, especially in the case of
> CHAR/VARCHAR/TEXT and BINARY data. Your suggestion is a halfway house,
> whereby tds_convert() writes to the calling program's 
> variables and the
> API clears up the mess afterwards. I think that's wrong 
> architecturally,
> messy and harder to debug when something goes wrong. As an 
> example of what
> could go wrong, you can't possibly have the case where 
> "tds_convert should
> fill destination buffer until filling them" If you pass in a desten of
> 2Gb!
> 

Perhaps I misunderstand it but I tought the problem was only on
char/varchar/text. If we document the behaviour of tds_convert I think this
is not a problem.

Well, I'll explain my idea on char a bit deeper.
tds_convert should return chars of string bounding to destlen without
terminating or padding in any case.
If we convert "test" to char(2) it should return (on the buffer) just "te"
(or best, in C { 't', 'e' }, not terminating). 
If we have dblib with destlen -1 we call tds_convert with destlen = 2G.
tds_convert copy only the bytes of the string (and return length copied).
Also tds_convert should return length of full string. 
If we convert string "quite long string" to a char(7) we return "quite l"
(not terminated) and a length of 17. ODBC for example can terminate with
"quite ", return full length and success while ctlib can return failure...

> 
> > My intention for 0.6 release was a convert.c without any 
> malloc in it (as
> > you can note applying my patches). I disappoint with this patch. It
> > reintroduce a lot of untested malloc (with relative core 
> dumps...). It also
> > lower performance by copy 2 time buffer for CHAR/VARCHAR...
> > 
> 
> I'm not sure about performance issues. You may be right, but 
> I would have
> thought the overhead was minimal. Untested ? all I can say is 
> that I've
> tried my best to test all the possibilities. tds_convert() 
> mallocs storage
> where the target type is CHAR/VARCHAR/TEXT/BINARY/IMAGE . As 
> long as the
> API always frees the pointers when this is the case, I think 
> it's pretty
> watertight. Hang on - I've found one case where this doesn't 
> happen - I'll
> fix that today :-)
> 

Problem is not free, is malloc.
>From convert.c:
           cr->c = malloc(srclen + 1);
           memset(cr->c, '\0', srclen + 1);
If malloc return NULL the program core dump...

> > 
> > Someone also should write tests for all library conversion.
> > 
> 
> They should, but it's a big job! we need a unittest for 
> dblib, ctlib and
> odbc! and the combinations are MANY.
> 

Mmm... I this that a unique source should check all api...

> Thanks for your feedback, and Regards,
> 
> Bill
> 

Mail are just a bit slow to speak...
What on #freetds on openprojects ??

bye
  freddy77

=================================
"STRICTLY PERSONAL AND CONFIDENTIAL

This message may contain confidential and proprietary material for the sole
use of the intended recipient. Any review or distribution by others is
strictly prohibited. If you are not the intended recipient please contact
the sender and delete all copies.
The contents of this message that do not relate to the official business of
our company shall be understood as neither given nor endorsed by it."

=================================