Re: [PATCH 2/4] http: drop support for curl < 7.16.0

[Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de>]

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
>>>> -#else
>>>> -#define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD
>>>> -#endif
>>>> -
>>> 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:
>>
>> https://curl.haxx.se/libcurl/c/CURLOPT_KEYPASSWD.html
>> 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.
>
> -Peff
It seems to be a decimal/hex issue:
docs/libcurl/symbols-in-versions:153:CURLOPT_KEYPASSWD               7.17.0

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.

Nicolas