On Tue, 25 Dec 2007, Dmitry Kurochkin wrote:

> I have started working on it. The attached patch is my first attempt
> to reuse connection before it is fully established.

Thanks a lot for your work on this!

First nit is that it doesn't apply on current CVS. (Quite easy to fix manually
though.)

> The patch is really small. I removed relevant check from ConnectionExists()
> and added condition for reuse case in CreateConnection(). So the second
> connection goes straight to DOWAIT state.

Your changes in there don't make much sense to me:

First, you removed the echeck for a non-complete name resolve. Why? When the
name isn't resolved, there won't be any check->sock[FIRSTSOCKET] filled in to
use anyway!

Then you removed the check for the "check->bits.close" which is TRUE when the
downloaded file is about to be closed after this retrieval. Such connections
are not suitable for trying a pipeline with anyway so I don't agree with the
removal.

Your commenting out parts of the logic in Curl_connect() is also a bit strange
to me since the comments explain the current reasoning and your new code
disagrees with the comments?! Can you elaborate on the new functionality by
making sure the comments are correctly describing the code?

What kind of tests/builds have you done with your patch?

> BTW At line 2432 I changed check->bits.proxy to httpproxy. I am behind a
> proxy at work and set http_proxy env var. detect_proxy() does not set proxy
> bit but sets httpproxy. So this check fails. I think this should be fixed in
> detect_proxy() to set proxy bit as well. But I was not sure so put this
> little hack :)

Yes it should, as this fix is incorrect and breaks re-use of connections using
other proxies than http. I committed a fix just now based on your findings,
thanks!

-- 
  Commercial curl and libcurl Technical Support: http://haxx.se/curl.html