On Wed, Mar 14, 2018 at 09:56:06PM +0000, Ramsay Jones wrote: > Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> > --- > > Hi Junio, > > I happened to be building git on an _old_ laptop earlier this evening > and gcc complained, thus: > > CC http.o > http.c:77:20: warning: ‘curl_no_proxy’ defined but not used [-Wunused-variable] > static const char *curl_no_proxy; > ^ > The version of libcurl installed was 0x070f04. So, while it was fresh in my > mind, I applied and tested this patch. Makes sense. This #if would go away under my "do not support antique curl versions" proposal. I haven't really pushed that forward since Tom Christensen's patches to actually make the thing build (and presumably since he is building on antique versions he can't turn on -Werror anyway, since IIRC it tends to have some false positives). I agree with Jonathan that this explanation should be in the commit message. The patch itself looks OK, although: > diff --git a/http.c b/http.c > index 8c11156ae..a5bd5d62c 100644 > --- a/http.c > +++ b/http.c > @@ -69,6 +69,9 @@ static const char *ssl_key; > #if LIBCURL_VERSION_NUM >= 0x070908 > static const char *ssl_capath; > #endif > +#if LIBCURL_VERSION_NUM >= 0x071304 > +static const char *curl_no_proxy; > +#endif > #if LIBCURL_VERSION_NUM >= 0x072c00 > static const char *ssl_pinnedkey; > #endif > @@ -77,7 +80,6 @@ static long curl_low_speed_limit = -1; > static long curl_low_speed_time = -1; > static int curl_ftp_no_epsv; > static const char *curl_http_proxy; > -static const char *curl_no_proxy; I'm not sure whether our ordering of these variables actually means much, but arguably it makes sense to keep the proxy-related variables near each other, even if one of them has to be surrounded by an #if. I guess you were going for ordering the #if's in increasing version order. I'm not sure the existing code follows that pattern very well. -Peff
