Re: [PATCH 1/2] http: allow selection of proxy authentication method

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2015, 19:42 UTC
Message-ID: <xmqqlhada6v8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1446628405-8070-2-git-send-email-k.franke@science-computing.de>

Knut Franke <k.franke@science-computing.de> writes:

Show 9 quoted lines

> diff --git a/http.c b/http.c
> index 7da76ed..a786802 100644
> --- a/http.c
> +++ b/http.c
> @@ -305,6 +326,40 @@ static void init_curl_http_auth(CURL *result)
>  #endif
>  }
>  
> +/* assumes *var is free-able */

This is not just "assumes", but it is wrong for the callers to pass unfreeable memory, so

	/* *var must be free-able */

> +static void var_override(const char **var, char *value)
> +{
> +	if (value) {
> +		free((void*) *var);

There may be a similar whitespace damage but I happened to notice this one.

	free((void *)*var);

> +static void init_curl_proxy_auth(CURL *result)
> +{
> +	var_override(&http_proxy_authmethod, getenv("GIT_HTTP_PROXY_AUTHMETHOD"));

If your libcurl does not understand CURLOPT_PROXYAUTH, do you need to do this var_override()? Shouldn't this be inside the #if..#endif below?

Show 8 quoted lines

> +
> +#if LIBCURL_VERSION_NUM >= 0x070a07 /* CURLOPT_PROXYAUTH and CURLAUTH_ANY */
> +	if (http_proxy_authmethod) {
> +...
> +	else
> +		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> +#endif
> +}

Other than that, looks cleanly done.  Thanks.