[freetds] pbcb: replacing strcpy(3)

Lowden, James K LowdenJK at bernstein.com
Thu May 8 16:57:59 EDT 2003


> From: Frediano Ziglio [mailto:freddyz77 at tin.it]
> Sent: May 8, 2003 11:01 AM
> 
> 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...

I'm sure you're right on all counts.  Still, to handle *any* USC-2 data, I'd
like to have a counted string library.  

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

> > 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... 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>)".  

> 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 

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

Regards, 

--jkl


The information contained in this transmission may contain privileged and
confidential information and is intended only for the use of the person(s)
named above.  If you are not the intended recipient, or an employee or agent
responsible for delivering this message to the intended recipient, any
review, dissemination, distribution or duplication of this communication is
strictly prohibited. If you are not the intended recipient, please contact
the sender immediately by reply e-mail and destroy all copies of the
original message. Please note that we do not accept account orders and/or
instructions by e-mail, and therefore will not be responsible for carrying
out such orders and/or instructions.





More information about the FreeTDS mailing list