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

[Jeff King <peff@peff.net>]

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

> 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().

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