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
