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:

quoted

quoted

quoted

-#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