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

From: Mark Lodato <lodatom@gmail.com>
Date: Jun 12, 2009, 23:13 UTC
Message-ID: <ca433830906121613y68e5bdax5778867c41b00339@mail.gmail.com>
In-Reply-To: <7vprdaarka.fsf@alter.siamese.dyndns.org>

On Fri, Jun 12, 2009 at 2:34 AM, Junio C Hamano<gitster@pobox.com> wrote:

Show 35 quoted lines

> Mark Lodato <lodatom@gmail.com> writes:
>
>> @@ -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.

I realized this after I submitted the patch. Locally I have modified my version to do something similar to the above, but checking libcurl versions rather than checking the existence of the macros (which don't exist, as Daniel pointed out.) If this patch series is accepted, I will make a cleaner version that includes this change.

Mark