Jeff King venit, vidit, dixit 13.06.2014 13:19:
> 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?
I sneekily fix this in 6/6... I thought 3/6 is on next already, too late
for a real v2. Otherwise I would put 6/6 before everything else.
>> + 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
Both of these issues actually come for following verify-tag as closely
as possible. If 3 is not applied already, I should do away with
sha1_object_info.
About the peeling I'm not so sure, since there's a difference between a
signed tag pointing to a commit and a signed commit. Since
verify-{tag,commit} are bare metal plumbing, I would expect callers to
use <rev>^{commit} explicitly if they don't care how <rev> peels to a
commit.
Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html