Re: [PATCH] Make verify-tag a builtin.

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Mon, 23 Jul 2007, Carlos Rica wrote:

> +static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
> +{
> +	struct child_process gpg;
> +	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
> +	char *path, *eol;
> +	size_t len;
> +	int fd, ret;
> +
> +	path = xstrdup(git_path("VTAG_TMP"));
> +	fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
> +	if (fd < 0)
> +		return error("could not create temporary file '%s': %s",
> +						path, strerror(errno));
> +	if (write_in_full(fd, buf, size) < 0)
> +		return error("failed writing temporary file '%s': %s",
> +						path, strerror(errno));
> +	close(fd);

I just tested something like "gpg --verify - - < <filename>" and it 
worked...

> +	/* find the length without signature */
> +	len = 0;
> +	while (len < size && prefixcmp(buf + len, PGP_SIGNATURE "\n")) {
> +		eol = memchr(buf + len, '\n', size - len);
> +		len += eol ? eol - (buf + len) + 1 : size - len;
> +	}

How about this instead:

	const char *sig = strstr(buf, "\n" PGP_SIGNATURE "\n"));
	if (!sig)
		sig = buf + len;

Hmm?

But of course only if gpg does not grok "git cat-file tag <signed-tag> |
gpg --verify - -".

> +	remove(path);

I think we use unlink() consistently, since we are sure it is no 
directory.

> +	if (type <= OBJ_NONE)
> +		return error("%s: bad object type.", name);
> +	if (type != OBJ_TAG)
> +		return error("%s: cannot verify a non-tag object of type %s.",
> +				name, typename(type));

These two can be unified into one, right?

> +int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> +{
> +	int i, verbose = 0, had_error = 0;
> +
> +	git_config(git_default_config);
> +
> +	if (argc == 1)
> +		usage(builtin_verify_tag_usage);
> +
> +	i = 1;

You can initialise i above.

> +	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
> +		verbose = 1;
> +		i++;
> +	}
> +
> +	signal(SIGPIPE, SIG_IGN);
> +	while (i < argc)
> +		if (verify_tag(argv[i++], verbose))

Heh.  I thought that you only made this a separate function to help 
building it into builtin-tag...

That was quick!

Ciao,
Dscho