cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Curl_read_plain and test 160

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Tue, 23 Sep 2008 22:47:58 +0200 (CEST)

On Tue, 23 Sep 2008, Dan Fandrich wrote:

> It's not just test 160, but tests 64, 69, 90 and 153 on several of my
> autobuilds that are now failing since this change went in (e.g.
> http://curl.haxx.se/auto/log.cgi?id=20080923094436-27242). The reason is
> because of differences in error handling with Curl_read_plain.

I did add code that now actually returns CURLE_RECV_ERROR if recv() returns -1
and errno is not one of the EAGAIN alternatives. I don't know why we never did
that in the past, I don't think that is the wrong things to do...

> This patch makes it closer to how it used to be:

> - if(ret)
> + if(ret < 0)

> and allows those tests to pass. However, that doesn't quite seem right
> since it ignores any actual errors and not just the EWOULDBLOCK condition.

Exactly, but that was what the old code did!

I had to disable test 160 since it now fails on me because now we detect the
error properly much earlier and thus the late detection an error that test 160
is about no longer happens. Possibly some or all of the failures you see are
similar? I'll have to check autobuild logs to figure these out!

> Also, the new code handles errors returned from Curl_sec_read differently
> (the big if(-1==nread) block is gone) which may have also caused a Kerberos
> regression.

I did read over the Curl_sec_read() code before I did the change, and I got
the impression that the errno stuff that followed basically was intended for
the plain socket case so I thought the removal of the code for the krb4 case
would be safe... I guess I could've screwed up and thus the EAGAIN things are
needed for the krb4 case of nread == -1 as well!

-- 
  / daniel.haxx.se
Received on 2008-09-23