Skip to Content.
Sympa Menu

freetds - RE: [freetds] pbcb: replacing strcpy(3)

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: "ZIGLIO Frediano" <Frediano.Ziglio AT vodafoneomnitel.it>
  • To: "FreeTDS Development Group" <freetds AT lists.ibiblio.org>
  • Subject: RE: [freetds] pbcb: replacing strcpy(3)
  • Date: Fri, 9 May 2003 11:42:10 +0200


> > > 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




Archive powered by MHonArc 2.6.24.

Top of Page