cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Fwd: [PATCH] check ip callback

From: Alexey Pesternikov <paster_at_page2rss.com>
Date: Fri, 14 Sep 2007 16:13:50 -0700

On 9/14/07, Dan Fandrich <dan_at_coneharvesters.com> wrote:
>
> On Fri, Sep 14, 2007 at 01:30:03PM -0700, Alexey Pesternikov wrote:
> > As soon as nobody concerned here is the new version. It uses modified
> > curl_sockopt_callback.
> > The calback can modify or refuse (by returning ) the address.
>
> Dont' forget to update the documentation accordingly.
>
> > typedef int (*curl_sockopt_callback)(void *clientp,
> > curl_socket_t curlfd,
> > - curlsocktype purpose);
> > + curlsocktype purpose,
> > + int* ai_family,
> > + int* ai_socktype,
> > + int* ai_protocol,
> > + struct sockaddr *ai_addr,
> > + socklen_t* ai_addrlen);
>
> Rather than passing these parameters individually, why not pass
> a pointer to a Curl_addrinfo? It would also need to be made public and
> therefore placed into curl/curl.h, which might be a bit tricky. In most
> cases it will end up being a typedef of the system's struct addrinfo,
> though.

I pointed that in one of my previous posts in the thread. The problem is
Curl_addrinfo depends on configuration built at compile time, and the
configuration is unavailable in runtime header (curl.h), which is right
thing. Otherwise it would make an unstable binary API.

My only other comment has to do with the semantics of the
> curl_sockopt_callback
> return code. Right now, libcurl will always bail if a nonzero is
> returned, i.e. an error, but if zero is returned, it will sometimes
> continue
> and sometimes redo the callback after reopening the socket (if the address
> family is detected to have changed). It seems somehow cleaner to me to
> have the callback make the determination as to whether it should close
> and reopen the socket (and call be called again), by returning a code
> other than 0 or 1.

That would make the code simpler and semantics cleaner, but breaks
compatibility. The man page clearly states: A non-zero return code from the
callback function will signal an unrecoverable error to the library and it
will close the socket and return CURLE_COULDNT_CONNECT. (Option added in
7.15.6.)
I will think twise before upgrading in the future if you guys take a
compatibility issues so easy :)

And since we're overloading the curl_sockopt_callback anyway and opening and
> closing sockets several times, what about providing a way for the callback
> function to provide its own open and connected socket for libcurl to use
> instead of the one libcurl wants? Is that overloading this poor callback
> function too much?

We already overloaded it to the point of making the callback name
misleading.

Thank you,
Alexey
Received on 2007-09-15