On Fri, Jun 27, 2014 at 03:18:36PM +0200, Michael J Gruber wrote:
> A wrong '}' made our code record the results of mergetag signature
> verification incorrectly.
>
> Fix it.
I think this is the right thing to do, but we went from:
if (...)
if (...) {
if (...)
...
else
...
}
to:
if (...)
if (...) {
if (...)
...
}
else
...
I think part of the point of the original {} was to make it more clear
which else went with which if. Perhaps we should use more here.
I also find the logic a bit hard to follow in that the outer conditions are:
if (needed_for_goodsig)
if (sig_is_not_good)
...
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.
-Peff
--
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