FreeTDS Development Group

[Frediano Ziglio <freddyz77 AT tin.it>]

Il gio, 2003-05-08 alle 16:02, Lowden, James K ha scritto:
> > From: Frediano Ziglio [mailto:freddyz77 AT tin.it]
> > Sent: May 8, 2003 2:24 AM
> > Il gio, 2003-05-08 alle 06:51, James K.Lowden ha scritto:
> > 
> > James, dstr_* was designed to be extended in such way...
> 
> Yes, I thought that might be the case.  Honestly, I don't like the opacity
> of DSTR, nor the casts, and I'm not convinced of the advantage of never
> pointing to NULL.  I realize those could all be fixed, but you seem to think
> they're good things.... :-)  I'd like a more transparent, less manhandled
> approach.  
> 

Encapsulation require information hiding... 

> Still, I think we should all discuss this.  However we address counted
> strings, they're going to be a pervasive metaphor, and it would be nice if
> we all liked the way they're done.  
> 
> I've thought many times of adding a C++ module to use basic_string<>.  That
> to me would be the most elegant approach, but at a heavy cost of
> dependencies and, I fear, portability.  I'm not aware of a generalized
> standalone counted string library for C, something that would happy dealing
> in UCS-2, and that ideally could engage iconv.  
> 

I don't think 0.62 can handle UCS-2 client... It's best to focus with
ASCII compatible charset... Some code require string parsing...

> > TDS_PBCB* pbcb_cpy (iconv_t cd, TDS_PBCB* dest, const TDS_PBCB* src);
> > TDS_PBCB* pbcb_strcpy (iconv_t cd, TDS_PBCB* pbcb, const char *src);
> > 
> > Why pass iconv_t just to copy a buffer?
> 
> Not just to copy a buffer, to convert a buffer.  If cd == -1, copy.  Else
> use iconv to convert from src to dest.  
> 

IMHO a bad idea. Copy is quite different in my mind from conversion...
Two separate functions are better.

> I think perhaps rather than returning a pointer, the functions should return
> inbytesleft from iconv (normally 0).  If nonzero, the caller can examine
> errno and decide what to do.  
> 

errno is not a good variable... in some system is not thread-safe. I
know iconv return error in errno however this error should be sudden
copied to another place. Best to return an error.

> > All I need (in query.c) is a function to return byte count of 
> > converted
> > string... Something like tds_get_converted_length(TDSICONVINFO * ic,
> > const char* s, size_t length);
> 
> We could do that, it's true.  The advantage of your idea is that there's no
> buffer allocation.  
> 
> I think it's a very loose design, though: here we use iconv to count the
> length of the converted string (discarding the output), there we call
> tds_put_string() and expect it to do the same conversion.  For every
> tds_put_string(), two independent calls to iconv() in two different
> functions.  
> 
> For all data except [n]text, the maximum buffer size is 8000 bytes; for
> metadata, 512 bytes.  For that reason, I would avoid malloc(3), favoring
> fixed buffers instead.  Let iconv convert once, preserve the output, send
> the length and the data as required by the protocol.  That would require
> e.g. tds_submit_query() to maintain 3 such buffers, but I think that's a
> good trade-off for the simplicity.  
> 
> Regards, 
> 

I know... however inserting image/text in a prepared statement require
(not in tds7+) to build an insert query. In this case insert a 1mb (just
like a big image) nowadays FreeTDS need:
- 1mb client data
- 1mb param data in tds results
- 2mb built query (0x123...)
- 2mb converted query
just 6 mb to insert only one (I don't think there is a worst case in all
FreeTDS). In this case pre-compute of length can save 2mb (integration
of odbc emulated prepare directly in libTDS can save 3mb).

So yes, we can handle conversion directly for the most cases but we
should be prepared for large buffers too...
Allocation 16K on stack is a bad idea. Although most system have large
stacks using dynamic memory and page of 4k can lead to segmentation...
Also we must convert more than one buffers at a time (to build a result
we need to convert all column names).

freddy77