cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: small addition to easy.c

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Fri, 16 Dec 2005 12:09:06 +0100 (CET)

On Fri, 16 Dec 2005, Vadim Lebedev wrote:

> We had to modify a little bit the lib/easy.c file to recover the socket CURL
> created for transaction I wonder if curl maintaners will accept following
> small addition to the lib/easy.c

Hello! I'm happy you've found use for libcurl in your project and that you're
willing to work with us to improve libcurl and make it work nicer with your
requirements and demands.

I don't have any particular objections against the actual purpose of this
change, but the way you did it will easily break!

Also, please consider providing at least some basic documentation for the new
feature (this low level extraction will need some careful notes about what you
actually get and what can go wrong when you attempt to do this). Of course you
get the golden plate if you provide a set of test cases too...

> /* * curl_easy_get_sock() is an external interface that allows an app to get
> * the socket of a session handle. */

> #ifndef WIN32
> typedef int SOCKET;
> #endif

To start with, I am against this typedef. Mostly because we already have a
typedef interally for the socket type that is used all over and introducing a
new one will only add confusion with no additional benefit. (And I intend to
have that typedef go public in the future when I provide the upcoming
curl_multi_socket() API.)

> CURL_EXTERN SOCKET curl_easy_get_sock(CURL *curl) {

I'm against adding a totally new function for just extracting information
after a performed request, especially since we already have a general purpose
function that does exactly that: curl_easy_getinfo(). I would rather like to
see that function get an additional CURLINFO_* type added.

> if (data->info.httpproxycode != 200)
> return -1;

Why would extracting the low leve socket have to bother about if the recent
HTTP code is 200 or not? That seems like logic for your application, not for
this function.

Not to mention that -1 is not a "proper" value for a function that returns
SOCKET since socket is an unsigned variable in Windows.

> return (SOCKET) data->state.connects[0]->sockfd;

This assumption is very naive and plain wrong in many cases. You can't be sure
that this socket is actually the last used one. You need to check for the last
used one in the "connection cache" and return that one.

-- 
  Commercial curl and libcurl Technical Support: http://haxx.se/curl.html
Received on 2005-12-16