On Mon, 16 Jan 2006, Allen Chan wrote:

Thanks for the patch! You'll find my comments and remarks on it below.

> Included is the patch I made with unidiff using winmerge. Browsing the file
> casually, you might see lines that were removed and then immediately added,
> could this possibly be differences in the EOL?

Most likely, yes. I also noticed you modified a few lines to use TABs instead
of the previously used spaces for indenting...

> I hope that you find the changes as useful as I did, as meager as they are.
> I would be very grateful for any suggestions and criticisms.

Let me first request that you don't use TABs and that you make your source
code no wider than 80 columns, as that's the style we use in curl.

On the actual function of the patch:

1 - You left some printf() calls in there that I trust you used for debugging
     but they can't be in production code. I suggest you turn them into infof()
     calls instead.

2 - CURLINFO_LOCAL_PORT. As Karl M pointed out, what's the point of having
     libcurl do getsockname() on the socket when your app simply can first
     extract the socket and then do getsockname() on it on its own? Are there
     really situations when you want to extract the local port only without
     getting the socket/connection ?

3 - CURLINFO_SOCKET suffers from the same flaw(s) that I already mentioned to
     Vadim Lebedev when he posted his patch: the code assumes that the
     connection with the latest time stamp is the most recently used. That is
     not true if the most recently used one in fact was closed, while the
     second to last one was not. It needs a somewhat more robust handling and I
     already suggested that it could be made by having the SessionHandle struct
     keep info about the most recently used connection or similar.

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