FreeTDS Development Group

["ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>]

> > 
> > >From read_and_convert
> > 
> > suppress->einval = *wire_size > 0; /* EINVAL matters only 
> on the last
> > chunk. */
> > if (-1 == tds_iconv(tds, iconv_info, to_client, &bufp, 
> > &bufleft, outbuf,
> > outbytesleft)) {
> >     tdsdump_log(TDS_DBG_NETWORK, "%L Error: 
> > read_and_convert: tds_iconv
> > returned errno %d\n", errno);
> >     if (errno != EILSEQ) {
> >             tdsdump_log(TDS_DBG_NETWORK, "%L Error: 
> > read_and_convert: "
> >                                          "Gave up 
> > converting %d bytes due to error %d.\n", bufleft,
> > errno);
> ...
> > note errno != EILSEQ
> 
> Noted.  I think it's a mistake, too.  We should test for 
> EINVAL, because we're looking for an incomplete (not illegal) 
> sequence.  That's why tds_put_string()'s test is different.  
> 
> I looked at read_and_convert() when I patched 
> tds_put_string() (as you might be able to tell).  I didn't 
> change it, because I was trying to fix just that one bug.  
> 

However we just report errors in dump so there are no problems at all...
There is a slightly difference between read_and_convert and
tds_put_string. When tds_iconv return E2BIG this is an error for
read_and_convert (it should never happen cause we always pass sufficient
buffers) but not for tds_put_string (it just means that we should flush
to wire and loop again). EILSEQ is returned only if a message is raised
(inside tds_iconv) so IMHO this is the real intention of errno != EILSEQ
test. A note about tds_iconv and results: results is not correct. We do
not compute total unconvertible character (we should sum all
irreversible results) and it's impossible to count unconvertible
character on error (cause iconv returns -1). We use tds_iconv result
just to test error so this is not a problem however we should not state
that we return "number of irreversible conversions performed"... just we
can't...

> > Another problem is wire state. If we start sending a string (in this
> > case a query) and we stop in the middle what should we do?
> 
> I think it's OK, more or less.  tds_put_string() only stuffs 
> text into a packet; it doesn't set up the packet or flush it. 
>  If it writes less text than it was asked to, the text 
> arriving at the server will be shorter than was intended.  
> The state will be fine, although it would be better not to 
> send the string at all.  
> 
> More worrisome:  Suppose tds_put_string() gets a really big 
> input, say 10,000 bytes, that overlaps several packets.  
> Suppose the first packet is sent OK (last packet indicator 
> 0x00), and we hit an EILSEQ partway through Packet 2.  What 
> now?  How to tell the server to ignore Packet 1?  
> 
> I don't know of any way to retract packets after they've been sent.  
> 

I don't think is even possible :(...
Time ago I think about the possibility to extend packet behond is limit
and use a faster way to write packet. That is tell to reserve some space
and then build packet using simpler pointers. Assuming we want to ouput
a 3 byte we just say

void *p = tds_put_reserve(tds, 3);
if (!p) error...
TDS_PUT_FAST_BYTE(tds, p, XXX);
TDS_PUT_FAST_BYTE(tds, p, YYY);
TDS_PUT_FAST_BYTE(tds, p, ZZZ);

where TDS_PUT_FAST_BYTE can be somethink like
#define TDS_PUT_FAST_BYTE(tds, p, data) do { *(((TDS_UCHAR*)p)++) =
data; } while(0)

We can extend this use saying: cache all bytes I send. tds_put_string
will just write to a cache and tds_flush/write_packet flush to socket.
This way we can even change query.c to output conversions to this
packet. 

Well, I clash a lot of ideas:
1- stream support
2- inlining simpler write
3- cache conversions
4- avoid buffers problems converting

1- as you know the support in libTDS of stream, be able to pass stream
instead of memory buffers
2- as you can see from macro many times we pack bytes/small integer
calling a lot of functions. Preallocating buffers and using
pointers/macro can reduce CPU use (the true bottleneck is however wire).
Using Intel platform (with unaligned memory access and little-endian)
code can be reduced a lot.
3- query.c do a lot of allocations/conversion. It would be good if we
can cache at wire level and then fill length as we proceed. You can note
that we allocate/convert all strings before just to avoid your problem
(sending partial queries to server)
4- tds_iconv/iconv expect to output characters, not bytes so you can't
split a character in two buffer (for example two packet buffers) so
extending packet you can avoid this problem.

freddy77