Re: [PATCHv2 3/6] verify-commit: scriptable commit signature verification

[Jeff King <peff@peff.net>]

On Fri, Jun 13, 2014 at 12:42:45PM +0200, Michael J Gruber wrote:

> +
> +	free(signature_check.gpg_output);
> +	free(signature_check.gpg_status);
> +	free(signature_check.signer);
> +	free(signature_check.key);
> +	return signature_check.result != 'G';
> +}

How about .payload here?

> +	type = sha1_object_info(sha1, NULL);
> +	if (type != OBJ_COMMIT)
> +		return error("%s: cannot verify a non-commit object of type %s.",
> +				name, typename(type));
> +
> +	buf = read_sha1_file(sha1, &type, &size);
> +	if (!buf)
> +		return error("%s: unable to read file.", name);

I think you can drop the sha1_object_info call and just check "type"
from read_sha1_file (they _should_ agree, but if they do not, I'd rather
pay attention to the one that came along with the buffer). And this is
the uncommon error path, so expanding the object into memory before we
die is not a big deal.

Should this peel to a commit if given a tag? I'd say probably. I know
you raised the issue elsewhere of keeping things simple, but I think if
you are calling verify-commit, you know you want a commit, and we should
treat the argument as a commit-ish. Anyway, if you go that route, then
lookup_commit_or_die is probably what you want.

Also, minor nit, but we typically do not end the error messages with a
full stop (we've been rather inconsistent in the past, but these days
seem to mostly settle on no punctuation).

-Peff