Jeff King <peff@xxxxxxxx> writes:
> There was a patch at the start of this thread, but it specifically
> checks for "sigc->result == U". That's probably OK, since I think it
> restores the behavior in earlier versions of Git. But I wonder if we
> should simply be storing the fact that gpg exited non-zero and relaying
> that. That would fix this problem and truly make the rule "if gpg
> reported an error, we propagate that".
Yeah, I like that. Something like this, perhaps? Points to note:
* status gets the return value from verify_signed_buffer(), which
essentially is what wait_or_whine() gives us for the "gpg
--verify" process.
* Even if status says "failed", we still need to parse the output
to set sigc->result. We used to use sigc->result as the sole
source of our return value, but now we turn 'status' into 'bad'
(i.e. non-zero) after parsing and finding it is not mechanically
good (which is the same criteria as we have always used before).
An already bad status is left as bad.
* And we return 'status'.
If we choose to blindly trust the exit status of "gpg --verify" and
not interpret the result ourselves, we can lose the "smudge status
to be bad if not G/U" bit, which I offhand do not think makes much
difference either way. I just left it there because showing what
can be removed and saying it can be dropped is easier than showing
the result of removal and saying it can be added--simply because I
need to describe "it" if I go the latter route.
gpg-interface.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 09ddfbc267..2e0885c511 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -67,26 +67,27 @@ static void parse_gpg_output(struct signature_check *sigc)
int check_signature(const char *payload, size_t plen, const char *signature,
size_t slen, struct signature_check *sigc)
{
struct strbuf gpg_output = STRBUF_INIT;
struct strbuf gpg_status = STRBUF_INIT;
int status;
sigc->result = 'N';
status = verify_signed_buffer(payload, plen, signature, slen,
&gpg_output, &gpg_status);
if (status && !gpg_output.len)
goto out;
sigc->payload = xmemdupz(payload, plen);
sigc->gpg_output = strbuf_detach(&gpg_output, NULL);
sigc->gpg_status = strbuf_detach(&gpg_status, NULL);
parse_gpg_output(sigc);
+ status |= sigc->result != 'G' && sigc->result != 'U';
out:
strbuf_release(&gpg_status);
strbuf_release(&gpg_output);
- return sigc->result != 'G' && sigc->result != 'U';
+ return !!status;
}
void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
