Re: [PATCH] log: correctly identify mergetag signature verification status
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:52
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] log: correctly identify mergetag signature verification status · Jeff King <hidden>
- 2016-06-15 · [PATCH] log: correctly identify mergetag signature verification status · Michael J Gruber <hidden>
Jeff King [off-list ref] writes:
Perhaps it would be easier to read (and would have made the logic error
you are fixing more obvious) as:
if (extra->len > payload_size) {
if (!verify_signed_buffer(...))
status = 0; /* good; all other code paths leave it as -1 */
else if (verify_message.len <= gpg_message_offset)
strbuf_addstr(&verify_message, "No signature\n");
}
That is, for each conditional to check one more thing needed for a good
signature, and then know that all other code paths leave status as -1.Thanks. Let's do it this way, then. log-tree.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 1982631..b4bbfe1 100644
--- a/log-tree.c
+++ b/log-tree.c@@ -446,16 +446,17 @@ static void show_one_mergetag(struct rev_info *opt, payload_size = parse_signature(extra->value, extra->len); status = -1; - if (extra->len > payload_size) - if (verify_signed_buffer(extra->value, payload_size, - extra->value + payload_size, - extra->len - payload_size, - &verify_message, NULL)) { - if (verify_message.len <= gpg_message_offset) - strbuf_addstr(&verify_message, "No signature\n"); - else - status = 0; - } + if (extra->len > payload_size) { + /* could have a good signature */ + if (!verify_signed_buffer(extra->value, payload_size, + extra->value + payload_size, + extra->len - payload_size, + &verify_message, NULL)) + status = 0; /* good */ + else if (verify_message.len <= gpg_message_offset) + strbuf_addstr(&verify_message, "No signature\n"); + /* otherwise we couldn't verify, which is shown as bad */ + } show_sig_lines(opt, status, verify_message.buf); strbuf_release(&verify_message);