Re: [PATCH 1/2] http.c: prompt for SSL client certificate password

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 12, 2009, 06:34 UTC
Message-ID: <7vprdaarka.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1243480563-5954-1-git-send-email-lodatom@gmail.com>

Mark Lodato <lodatom@gmail.com> writes:

Show 14 quoted lines

> @@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)
>  
>  	if (ssl_cert != NULL)
>  		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
> +	if (has_cert_password())
> +		curl_easy_setopt(result,
> +#if LIBCURL_VERSION_NUM >= 0x071700
> +				 CURLOPT_KEYPASSWD,
> +#elif LIBCURL_VERSION_NUM >= 0x070903
> +				 CURLOPT_SSLKEYPASSWD,
> +#else
> +				 CURLOPT_SSLCERTPASSWD,
> +#endif
> +				 ssl_cert_password);

This is purely style and readability, but if you do something like this much earlier in the file:

    #if !defined(CURLOPT_KEYPASSWD)
    # if defined(CURLOPT_SSLKEYPASSWD)
    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLKEYPASSWD
    # elif defined(CURLOPT_SSLCERTPASSWD
    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLCERTPASSWD
    # endif
    #endif

you can write your main codepath using the latest cURL API without ifdef. The callsite can simply say:

	if (must_set_cert_password())
        	curl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);

which I think would be much easier to follow.

This assumes that KEYPASSWD is the latest API, and in older versions only names are different, which your code implies. I have a vague recollection that SSLCERTPASSWD actually deprecated KEYPASSWD (i.e. your #if...#endif chain is wrong), but I didn't actually check the cURL documentation [*1*] to see if that is the case.

[Reference]

*1* http://cool.haxx.se/cvs.cgi/curl/docs/libcurl/symbols-in-versions?rev=HEAD