[freetds] pbcb: replacing strcpy(3)
Frediano.Ziglio at vodafoneomnitel.it
Fri May 9 05:42:10 EDT 2003
> > > 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
> > Two separate functions are better.
> I see where you're coming from. My thought is that we have a
> 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)
> 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_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
> > 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)
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
> 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...
More information about the FreeTDS