Re: [PATCH 2/5] http: correct curl version check for CURLOPT_PINNEDPUBLICKEY

On Wed, Sep 08, 2021 at 05:31:53PM +0200, Ævar Arnfjörð Bjarmason wrote:

In aeff8a61216 (http: implement public key pinning, 2016-02-15) a
dependency and warning() was added if curl older than 7.44.0 was used,
but the relevant code depended on CURLOPT_PINNEDPUBLICKEY, introduced
in 7.39.0.

According to the manpage for CURLOPT_PINNEDPUBLICKEY, it looks like
support for various formats and implementations was phased in. In
particular, 7.44.0 picked up sha256 support (I guess for a fingerprint?
I've never used this feature) for most major implementations.

But in terms of compiling, all we care about is that the constant is
there. So I think the cutoff point you found is what we want. Presumably
when the file format isn't supported we'd get some error, though it's
not clear if that would come during the actual curl_*_perform(), or if
we should be checking the curl_easy_setopt() result.

Let's also remove the macro check before we declare the ssl_pinnedkey
variable, the pattern for other such variables is to declare the
static variable unconditionally, we just may not use it on older
versions. This reduces macro verbosity.

The reduction in verbosity comes at the small cost of issuing a
warning about the unused variable if this code is compiled with curl
versions older than 7.39.0. I think that's an acceptable trade-off,
anyone compiling a new git with a 2014-era toolchain likely has at
least other warning that'll have prompted them not to use -Werror, and
if not maybe this'll prompt them to compile their new git with a more
modern libcurl.

OK. That's a bit of a departure from how we've handled variables before,
but it does make the code a bit cleaner. And I am fine with the attitude
of "if you are using ancient tools, you may see some extra warnings". We
already know this is true for older compilers, and it's not worth caring
too much about.

-Peff