Re: [PATCH 2/4] http: drop support for curl < 7.16.0
- Date: Wed, 9 Aug 2017 23:29:30 +0200
- From: Nicolas Morey-Chaisemartin <NMoreyChaisemartin@xxxxxxx>
- Subject: Re: [PATCH 2/4] http: drop support for curl < 7.16.0
Le 09/08/2017 à 23:17, Jeff King a écrit :
> On Wed, Aug 09, 2017 at 08:03:05PM +0200, Nicolas Morey-Chaisemartin wrote:
>>>> -#if LIBCURL_VERSION_NUM >= 0x071700
>>>> -/* Use CURLOPT_KEYPASSWD as is */
>>>> -#elif LIBCURL_VERSION_NUM >= 0x070903
>>>> -#define CURLOPT_KEYPASSWD CURLOPT_SSLKEYPASSWD
>>>> -#define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD
>>> This part I am not sure. Don't we still need to substitute
>>> CURLOPT_KEYPASSWD with CURLOPT_SSLKEYPASSWD for versions below
>>> 071700, e.g. 071000 which is 7.16.0?
>> According to the documentation:
>> This option was known as CURLOPT_SSLKEYPASSWD up to 7.16.4 and
>> CURLOPT_SSLCERTPASSWD up to 7.9.2.
>> So the patch breaks things (broken for 7.16.[0-4]). But the series
>> does not as the next patch ensure at least 7.19.4
> But the #ifdef above says 071700, which is 7.23.0. I wonder if we just
> got it wrong back then (maybe hex confusion with 7.17.0?). I have a
> build setup for old versions of curl, so I'll double-check that 7.19.4
> builds with KEYPASSWD. And dig in the history to see if there's any
> comment on this mismatch.
It seems to be a decimal/hex issue:
I guess it should still work because it is now defined like this:
curl.h:#define CURLOPT_SSLKEYPASSWD CURLOPT_KEYPASSWD
If I'm not mistaken on cpp behaviour it means CURLOPT_KEYPASSWD is evaluated to CURLOPT_SSLKEYPASSWD (git define) which is evaluated into CURLOPT_KEYPASSWD (curl define).
It should stop here as CURLOPT_KEYPASSWD was not a defined macro when the curl one was evaluated.
It might be worth cleaning though, specially it wouldn't work anymore if the git macro is ever moved before the curl include.