Jeff King <peff@xxxxxxxx> wrote: > On Mon, Oct 14, 2013 at 11:38:39PM +0000, Eric Wong wrote: > > > I wanted it to work as older curl first (since I noticed this > > on an old server). But your patch on top of mine looks reasonable, > > thanks. > > Makes sense. Here it is with a real commit message (on top of the > ew/keepalive topic). > > -- >8 -- > Subject: http: use curl's tcp keepalive if available > > Commit a15d069 taught git to use curl's SOCKOPTFUNCTION hook > to turn on TCP keepalives. However, modern versions of curl > have a TCP_KEEPALIVE option, which can do this for us. As an > added bonus, the curl code knows how to turn on keepalive > for a much wider variety of platforms. The only downside to > using this option is that not everybody has a new enough curl. > Let's split our keepalive options into three conditionals: > > 1. With curl 7.25.0 and newer, we rely on curl to do it > right. > > 2. With older curl that still knows SOCKOPTFUNCTION, we > use the code from a15d069. > > 3. Otherwise, we are out of luck, and the call is a no-op. > > Signed-off-by: Jeff King <peff@xxxxxxxx> Thanks. Tested-by: Eric Wong <normalperson@xxxxxxxx> on curl 7.21.0 and 7.26.0, confirmed via strace: setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, [1], 4) = 0 I can also confirm 7.26.0 adds: setsockopt(fd, SOL_TCP, TCP_KEEPIDLE, [60], 4) = 0 setsockopt(fd, SOL_TCP, TCP_KEEPINTVL, [60], 4) = 0 to the strace on Linux. > --- > Given the #ifdefs in curl's keepalive code, I suspect we may see build > problems for people in case 2 on some systems, with or without my patch. > I think this patch is a strict improvement, though; if they have a new > enough curl, they will not even look at the case 2 code. And if they do > not, our previous options were: > > a. Add platform-specific code for them. > > b. Tell them they are out of luck, and add an #ifdef to push them into > case 3. Case 2 works fine on my Debian Squeeze system. > Now we have an extra option: > > c. Tell them to upgrade curl. :) Yes, I keep forgetting I still have a Debian Squeeze machine :x -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html