FreeTDS Development Group

[Frediano Ziglio <freddyz77 AT tin.it>]

Il lun, 2004-01-26 alle 21:34, Frediano Ziglio ha scritto:
> > From: ZIGLIO, Frediano, VF-IT [mailto:Frediano.Ziglio AT vodafone.com]
> > Sent: January 26, 2004 11:14 AM
> > 
> > You are right however for 0.62 is better to have less changes, just to
> > avoid other possible bugs. Also in read.c (read_and_convert) we have
> > similar code but we test just EILSEQ and stop if there are no
> > conversions.
> > Thinking about tds_iconv suppress was introduced to avoid double
> > messages so it seems correct to return failure on EINVAL.
> 
> One of us is confused.  :-)
> 

None, I think :)

>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);
                tdsdump_log(TDS_DBG_NETWORK, "\tTroublesome bytes:\n\t%D\n", 
bufp,
bufleft);
        }

        if (bufp == temp) {     /* tds_iconv did not convert anything, avoid
infinite loop */
                tdsdump_log(TDS_DBG_NETWORK, "%L No conversion possible: 
draining
remaining %d bytes.\n", *wire_size);
                tds_get_n(tds, NULL, *wire_size); /* perhaps we should read
unconverted data into outbuf? */
                *wire_size = 0;
                break;
        }
                
        if (bufleft) {
                memmove(temp, bufp, bufleft);
        }
}

note errno != EILSEQ

>From tds_put_string

if (-1 == tds_iconv(tds, tds->iconv_info[client2ucs2], to_server, &s,
&inbytesleft, &poutbuf, &outbytesleft)){
                
        if (errno == EINVAL  && tds->iconv_info[client2ucs2]->suppress.einval 
)
{
                tdsdump_log(TDS_DBG_NETWORK, "%L tds_put_string: tds_iconv()
encountered partial sequence "
                                             "(anticipated).  %d bytes 
remain. Continuing.\n", 
                                             len + (int) inbytesleft);
        } else {
                /* It's not an incomplete multibyte sequence, or it IS, but 
we're not
anticipating one. */
                tdsdump_log(TDS_DBG_NETWORK, "%L Error: tds_put_string: "
                                             "Gave up converting %d bytes due 
to error %d.\n", 
                                             (int) inbytesleft, errno);
                tdsdump_log(TDS_DBG_NETWORK, "\tTroublesome bytes:\n\t%D\n", 
s, (int)
inbytesleft);
        }

        if (poutbuf == outbuf) {        /* tds_iconv did not convert 
anything, avoid
infinite loop */
                tdsdump_log(TDS_DBG_NETWORK, "%L Error: tds_put_string: No 
conversion
possible, giving up.\n");
                break;
        }
}

here we have errno == EINVAL

The reason is simple, we need to ignore EINVAL and E2BIG cause, as you
said below, we need to ignore these cases, catched by poutbuf == outbuf
test.

> EINVAL signifies an incomplete sequence at the end of the input buffer. 
> Because we arbitrarily carve the input into 128-byte chunks, we know
> that UTF-8 characters my be sliced in two at the boundary.  We should
> expect tds_iconv() to return EINVAL; we'll provide the rest of the
> sequence on the next call.  Part of the purpose of the suppress field
> was to avoid any message for conditions the caller would handle.  
> 
> As I said, though, the logic suboptimal.  There's no reason to carve up
> the input buffer.  We can let tds_iconv() try to fill the output buffer
> and, if it runs out of room (E2BIG), flush the output and continue. 
> That way, EILSEQ and EINVAL become real errors, and E2BIG is
> suppressed.  
> 

There was... there was no way to suppress warnings from tds_iconv so we
avoid E2BIG limiting input :)

> In tds_put_string(), we don't return immediately on, say, EILSEQ, but
> the effect is the same.  On the next iteration of the loop after EILSEQ,
> no bytes will be converted, poutbuf == outbuf, and we exit the loop (and
> procedure).  
> 
> In both tds_put_string() and read_and_convert(), we convert as much as
> we can, emitting an error message in the event we're unable to finish
> the job.  That's roughly the same behavior an application would see if
> the conversion were done on the server (a la Sybase).  I'm not sure what
> return code the application should see.  For example, does dbsqlexec()
> return FAIL if Sybase can't convert the string?  

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?

freddy77