FreeTDS Development Group

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

> 
> On Mon, 26 Jan 2004, "ZIGLIO, Frediano, VF-IT"
> <Frediano.Ziglio AT vodafone.com> wrote:
> > 
> > Patch works however it's the correct way to fix problem? We 
> ignore all
> > errors replacing with logs... 
> 
> The patch doesn't completely ignore all errors.  Errors are emitted by
> tds_iconv(), and can be trapped by the error handler.  
> 
> Perhaps you're right, though.  tds_put_string() simply 
> returns the count
> of bytes sent.  It used to return -1 if it encountered any 
> problem from
> tds_iconv().  Perhaps it should return the count of input 
> bytes converted:
> "len" (or strlen()) for success, and some lesser number for 
> failure.  That
> would give the caller more information.  
> 

There were a reason to return converted len; it was used to compute some
wire length. I looked throw code and is no used so returning void it's
the same... Callers use tds_put_string just to output strings with small
checks. If client needs more checks it should convert string before,
just to be sure it can convert all it wants (just like query.c usually
do).

> > Perhaps it should be better to return 0
> > from tds_iconv on EINVAL and suppress->einval ?? Or just 
> continue (in
> > write.c) if we get EINVAL and there is still data to write 
> (len > 0) ??
> 
> If tds_iconv() sets errno to EINVAL on any chunk except the 
> last one, we
> do and should ignore it.  The input buffer pointer & count 
> will be set to
> the beginning of the incomplete sequence; we just flush the output and
> call tds_iconv() again.  (In fact, we work too hard, estimating
> inbytesleft.  We should just suppress E2BIG and let 
> tds_iconv() convert as
> much of the input as it has room for.)  
> 

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.

> > IMHO this patch should be put on 0.62 too...
> 
> Will do.  
> 

Ok, I updated utf8_1 test (CVS HEAD) to test this case.

freddy77