cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] Keepalive - one step further

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Thu, 3 Jan 2008 23:10:44 +0100

On Jan 3, 2008 11:43 AM, <autrelandes-curl_at_yahoo.fr> wrote:

> > Thanks, but can you please elaborate on why you think libcurl needs to
> > have this ability rather than you just set these options in your app

> The parameters TCP_KEEPCNT, TCP_KEEPIDLE and TCP_INTVL are defined in
> <netinet/tcp.h>. This header is not available from src/main.c, while it is
> available from lib/connect.c.

Right, but

#1 - you can easily add the necessary includes to scr/main.c.

and

#2 - I'm a believer in letting the app do what it can as far as
possible for these kinds of edge cases where we know most applications
won't care about and for those that care it is very easy for the
application to provide the functionality

> I just applied the same rules which are used for the use of TCP_NODELAY in
> curl : TCP_NODELAY is also in <netinet/tcp.h> ; a function, tcpnodelay, has
> been defined in lib/connect.c.

Yes, but a mistake from the past shouldn't guide us for upcoming
things! If that change wouldn't come today, I would've suggested that
for a callback solution as well!

> Maybe the same include problem arises with the parameter IPPROTO_TCP (as one
> cannot use the SOL_SOCKET level for modifying the keepalive values ; see man
> 7 tcp and man 7 socket) which is defined in <netinet/in.h>, but I did not
> check.

Why is including other/new files in src/main.c a problem?

BTW, two nits on the actual patches:

o please use source code with no less than 80 characters per line

o please concatenate all separate files' patches (of course if they
all combined is one particular feature/fix/job) into one single file
when you submit them, as it makes it a lot easier for us to review the
lot at once.
Received on 2008-01-04