Re: [PATCH/RFC] builtin/tag.c: move PGP verification inside builtin.

[Santiago Torres <santiago@nyu.edu>]

Hi Jeff.

Sorry for the delay with this, I got caught up with coursework.

This is my first stab at this, in the dumbest/simplest way imaginable. I
don't like that there is no code reuse (the run_gpg_verify function is
repeated here and in the plumbing command). I would appreciate pointers
on what would be the best way to avoid this.

I also spent quite some time figuring out what you meant with

> Do note the trickery with SIGPIPE in verify-tag, though. We probably
> need to do the same here (in fact, I wonder if that should be pushed
> down into the code that calls gpg).
I don't see any explicit SIGPIPE trickery here. Any pointers?

Thanks!
-Santiago.


On Thu, Mar 24, 2016 at 05:39:20PM -0400, santiago@nyu.edu wrote:
> From: Santiago Torres <torresariass@gmail.com>
> 
> The verify tag function is just a thin wrapper around the verify-tag
> command. We can avoid one fork call by doing the verification instide
> the tag builtin instead.
> 
> Signed-off-by: Santiago Torres <santiago@nyu.edu>
> ---
>  builtin/tag.c | 44 ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 1705c94..be5d7c7 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -30,6 +30,27 @@ static const char * const git_tag_usage[] = {
>  
>  static unsigned int colopts;
>  
> +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
> +{
> +	struct signature_check sigc;
> +	int len;
> +	int ret;
> +
> +	memset(&sigc, 0, sizeof(sigc));
> +
> +	len = parse_signature(buf, size);
> +
> +	if (size == len) {
> +		write_in_full(1, buf, len);
> +	}
> +
> +	ret = check_signature(buf, len, buf + len, size - len, &sigc);
> +	print_signature_buffer(&sigc, flags);
> +
> +	signature_check_clear(&sigc);
> +	return ret;
> +}
> +
>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
>  {
>  	struct ref_array array;
> @@ -104,13 +125,24 @@ static int delete_tag(const char *name, const char *ref,
>  static int verify_tag(const char *name, const char *ref,
>  				const unsigned char *sha1)
>  {
> -	const char *argv_verify_tag[] = {"verify-tag",
> -					"-v", "SHA1_HEX", NULL};
> -	argv_verify_tag[2] = sha1_to_hex(sha1);
>  
> -	if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
> -		return error(_("could not verify the tag '%s'"), name);
> -	return 0;
> +	enum object_type type;
> +	unsigned long size;
> +	const char* buf;
> +	int ret;
> +
> +	type = sha1_object_info(sha1, NULL);
> +	if (type != OBJ_TAG)
> +		return error("%s: cannot verify a non-tag object of type %s.",
> +				name, typename(type));
> +
> +	buf = read_sha1_file(sha1, &type, &size);
> +	if (!buf)
> +		return error("%s: unable to read file.", name);
> +
> +	ret = run_gpg_verify(buf, size, 0);
> +
> +	return ret;
>  }
>  
>  static int do_sign(struct strbuf *buffer)
> -- 
> 2.7.3
>