Skip to Content.
Sympa Menu

freetds - Re: convert.c and the next release

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: Frediano Ziglio <freddyz77 AT tin.it>
  • To: TDS Development Group <freetds AT franklin.oit.unc.edu>
  • Subject: Re: convert.c and the next release
  • Date: 04 Aug 2002 10:11:32 +0200


Il dom, 2002-08-04 alle 01:28, James K.Lowden ha scritto:
> All,
>
> How has the experience been with Bill's new conversion stuff? Are we
> happy?
>
> I'm a little not, on principle. But if the new code works, I say it's
> high time we kicked it out the door.
>
> Bill, I'm just not fond of libraries allocating memory and expecting me to
> free it. I would actually prefer, as you said you don't, that tds_convert
> use an output buffer provided by the API, and let the API "clear up the
> mess afterwards".
>
> I can think of only two reasons for tds_convert to allocate memory:
>
> 1. The caller doesn't know how big the output buffer should be.
> 2. tds_convert wants to give the caller some choice about what to do with
> the output buffer in the event it's not big enough.
>
> I think case #1 never happens and case #2 is unimportant.
>
> Other opinions? Would anyone greatly mind if I slightly rewound Bill's
> logic? It can wait for 0.61.

Some (other) words: the problem is mainly on variable size (other are
fully binary compatible).
I think final solution for remove malloc/two copy problem is:

library:
test all parameter
call tds_convert
clear buffer returned

Parameter for tds_convert destination stuff should be:
- buffer
- length
- type
- flag to copy data if shorter buffer

I explain all that by examples.

cs_convert, conversion from int 12345 to CS_CHAR_TYPE
- destlen == -1, no adjustment
pass to tds_convert length == maximun (2GB on x86), type == TDSCHAR,
flag false
tds_convert fill buffer with "12345" (not terminated)
- destlen == 5, no adjustement
pass to tds_convert length == 5, type == TDSCHAR, flag false
tds_convert fill buffer with "12345" (not terminated)
- destlen == 4, no adjustement
pass to tds_convert length == 4, type == TDSCHAR, flag false
tds_convert return fail
- destlen == 5, null terminated
pass to tds_convert length == 4, type == TDSCHAR, flag false
(note that cs_convert pass 4 to tds_convert to reserve space for
terminator)
tds_convert return fail

conversion with odbc (have not adjustement, always terminated)
- destlen == 5
pass to tds_convert length == 4, type == TDSCHAR, flag true
tds_convert fill buffer with "1234" (not terminated) and return a
length of 5.

Rewound?? Bill fix also some things, other than change conversion stuff
(it also do some fix for it), so IMHO we have to work on CVS version and
change it. I agree for returning the old DBANY (now CONV_RES) to
library.

I think next release must slide a bit for finish this convert stuff. I'm
preparing a test for ctlib conversion and... it don't work very well...
Our test do not check for overflow (converting INT to SMALLINT for
example) and some conversion are missed (like INT to BIT) but
tds_willconvert return true on it.

>
> But that's a niggle. The really really great news AIUI is that we have a
> complete working datatype conversion matrix. Do I in fact understand
> correctly? (Curiously, it's not in the ChangeLog.)
>
> Frediano, you have several open patches posted. It looks like some of
> them are OBE (overcome by events, no longer needed). Could you take a
> look and tell me what's still needed, please?

#578441 should be merged while finish work on convert
#580072 is not only an optimization but fix some buffer overflow and
numeric overflow problem... as discussed this patch raise a problem...
MSSQL and Sybase seem to have different handle of numeric (byte order in
MSSQL is always little endian while Sybase seem to be always big endian)
I tested with MSSQL and is correct, I don't know about Sybase...
#580070 should be deleted (is only a #580070 duplicate)
#580913 is small, simple and required
#582163 is trivial too
#582424 perhaps can be a suggestion, but should be implemented is some
way...
#582445 another trivial patch...

Other can wait...

Note that last patch for NT Domain auth do not compile without
--with-ssl on configure. NT Domain stuff should be turned off if ssl if
not enabled.

regards,
freddy77






Archive powered by MHonArc 2.6.24.

Top of Page