> I know you are just copying this from the one in builtin/verify-tag.c,
> but I find the use of "size" and "len" for two different purposes
> confusing. Those words are synonyms, so how do the variables differ?
>
> Perhaps "payload_size", or "signature_offset" would be a better term for
> "len".
I agree, I'll give this a go.
>
> > + if (size == len) {
> > + write_in_full(1, buf, len);
> > + }
>
> If the two are the same, we have no signature. Should we be returning
> early, and skipping check_signature() in that case?
This makes sense, for both the builtin and the plumbing. Let me give
this a try.
> > @@ -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};
>
> So the original was passing "-v" to verify-tag. That should put
> GPG_VERIFY_VERBOSE into the flags field. But later:
>
> > + ret = run_gpg_verify(buf, size, 0);
>
> We don't pass any flags. Shouldn't this unconditionally pass
> GPG_VERIFY_VERBOSE?
>
Right, I missed this. Sorry about this.
> All of this seems like a repetition of verify_tag() in
> builtin/verify-tag.c (and ditto with run_gpg_verify()). Can we move
> those functions into tag.c and just call them from both places, or is
> there some difference that needs to be taken into account (and if the
> latter, can we refactor them to account for the differences?).
>
Yep, this is what was troubling me (as I mentioned on the followup). I
didn't want to remove the "static" classifier for the function (as there
could be a major reason for this decision).
If this last chage is ok with you I can send the fixed-up version right
away.
Thanks!
-Santiago.
--
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