FreeTDS Development Group

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

> > > 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 see where you're coming from.  My thought is that we have a 
> TDSICONVINFO
> array attached to the socket.  One of them represents
> "ascii2server_metadata" conversions.  For a server whose 
> metadata is ASCII,
> there are three choices:
> 
> 1.  maintain an iconv cd for ascii<->ascii, always convert.
> 2.  keep cd as -1, and
>     a.  write functions that detect the -1 and simply copy, or
>     b.  require caller to distinquish.  
> 
> In "2b" case, we write a lot of code like this:
> 
> if (thing->to_wire == (iconv_t) -1) 
>       strcpy(...)
> else
>       pbcb_strcpy(...)
> 
> I prefer to avoid the "2b" style.  (OK, I'll say it: I prefer 
> not 2b, if
> that's the question.)  In situations where conversion is 
> possible, it's
> better to treat a nonconverting copy as a trivial instance of 
> a converting
> copy.  For support, I rely on complexity analysis, which 
> finds that code
> defects rise with branches.  The 2b style forces many more 
> branches than 1
> or 2a. 
> 
> Of course, for cases where you *know* no conversion is 
> warranted, you still
> have memcpy(3) or some iconv-less library function at your disposal.  
> 

Well two function like

pbcb_copy(PBCB,PBCB)
pbcb_convert(iconv_t cd, PBCB,PBCB)  (handling cd == -1 like pbcb_copy)

Should fix both problems...

> > > 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.
> 
> I don't set errno; I call iconv(3), which sets errno.  I 
> can't change that
> behavior or improve on it.  The threadsafety of iconv's errno 
> interaction is
> no better or worse than for the system as a whole.  
> 

I know... in recent system errno is thread-safe (in Linux it's just a define 
like (*get_errno_p()) )... However we should design libTDS to be thread-safe 
in all system...

> > 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).
> 
> I thought we established that text/image data can't be 
> parameters to stored
> procedures and prepared statements.  
> 
> I can, however, imagine a client sending us a straight query, 
> "insert table
> values (<text>)".  
> 

Probably 0.62 will handle server prepared with TDS7+...

> > So yes, we can handle conversion directly for the most cases but we
> > should be prepared for large buffers too...
> 
> Agreed.  The fewer times the data are touched and copied, the 
> more efficient
> we are with the processor and memory, and the more likely are 
> the data to
> arrive safely.  
> 
> In TDS_PBCB, there are three members: pb, cb, and size.  
> Normally, cb <=
> size, meaning pb is OK.  We could have a function that "keeps going",
> completing the conversion and setting cb > size.  The caller 
> would then
> know: if cb <=size, use pb, else reconvert.  I don't know if 
> that's the best
> answer.  It seems like it might be helpful when the data 
> might be [n]text.
> (Image data should be passed verbatim, no iconv.)  
> 
> For tds_submit_query(), I can see how 
> 

There are a lot of functions that seem to do the same job. Convert string 
using chunk. Perhaps using callback can help. Think a function like this

typedef int (*tds_iconv_read)(void *param, char* buffer, size_t max_read);
typedef void (*tds_iconv_write)(void *param, char* buffer, size_t max_read);
void tds_convert(iconv_t cd, tds_iconv_read f_read, void * param_read, 
tds_iconv_write f_write, void * param_write)
{
        char buf[256];
        ...
        while(...) {
                ...
                f_read(param_read, buf, to_be_readed);
                ... (convert chunk) ...
                f_write(param_write, buf, to_be_written);
                ...
        }
        ...
}


Such a function can be used to:
- reading a file and filling a buffer (bulk)
- compute converted length (not silly conversions)
- read data from wire to tds result
- write data from result to wire
- ...

Just changing parameters...

> > tds_get_converted_length(TDSICONVINFO * ic, const char* s, 
> size_t length);
> 
> would work well.  OTOH, in login.c, we always know the buffer 
> sizes and
> don't need to be so conservative.  The same is true when 
> reading metadata
> for a result set.  
> 
> > 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...
> 
> Is that opinion generally held?  I don't think a 16 KB 
> character buffer on
> the stack is irresponsible.  
> 

However can core on some platform...

freddy77