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

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Nov 2015 11:42:51 -0800

Knut Franke <[email-masked]> writes:

> 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?

> +
> +#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.