On Fri, Jun 12, 2009 at 2:34 AM, Junio C Hamano<gitster@xxxxxxxxx> wrote: > Mark Lodato <lodatom@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html