Re: [PATCH v3 3/4] builtin/verify-tag: move verification code to tag.c

From: Jeff King <peff@peff.net>
Date: Apr 3, 2016, 04:45 UTC
Message-ID: <20160403044502.GC1519@sigill.intra.peff.net>
In-Reply-To: <1459638975-17705-4-git-send-email-santiago@nyu.edu>

On Sat, Apr 02, 2016 at 07:16:14PM -0400, santiago@nyu.edu wrote:

Show 6 quoted lines

> From: Santiago Torres <santiago@nyu.edu>
> 
> The PGP verification routine for tags could be accessed by other
> commands that require it. We do this by moving it to the common tag.c
> code. We rename the verify_tag() function to pgp_verify_tag() to avoid
> conflicts with the mktag.c function.

One nit: even though GPG is just an implementation of PGP, and technically the standard and formats are called PGP, we tend to name everything GPG in the code. So this probably should be gpg_verify_tag().

Show 14 quoted lines

> -	len = parse_signature(buf, size);
> -
> -	if (size == len) {
> -		if (flags & GPG_VERIFY_VERBOSE)
> -			write_in_full(1, buf, len);
> -		return error("no signature found");
> -	}
> [...]
> +	payload_size = parse_signature(buf, size);
> +
> +	if (size == payload_size) {
> +		write_in_full(1, buf, payload_size);
> +		return error("No PGP signature found in this tag!");
> +	}

I'm happy to see the more readable variable name here. I wonder if we should leave the error message as-is, though, as this is just supposed to be about code movement (and if we are changing it, it should adhere to our usual style of not starting with a capital letter, and not ending in punctuation).

-Peff