On Sun, Mar 31, 2013 at 05:03:44PM +0200, Thomas Rast wrote: > John Keeping <john@xxxxxxxxxxxxx> writes: > > > On Sun, Mar 31, 2013 at 04:33:57PM +0200, Sebastian Götte wrote: > >> diff --git a/commit.c b/commit.c > >> index eda7f90..bb2d9ad 100644 > >> --- a/commit.c > >> +++ b/commit.c > >> @@ -1029,6 +1029,8 @@ static struct { > >> } sigcheck_gpg_status[] = { > >> { 'G', "[GNUPG:] GOODSIG " }, > >> { 'B', "[GNUPG:] BADSIG " }, > >> + { 'U', "[GNUPG:] TRUST_NEVER" }, > >> + { 'U', "[GNUPG:] TRUST_UNDEFINED" }, > >> }; > >> > >> static void parse_gpg_output(struct signature_check *sigc) > >> @@ -1050,11 +1052,12 @@ static void parse_gpg_output(struct signature_check *sigc) > >> found += strlen(sigcheck_gpg_status[i].check); > >> } > >> sigc->result = sigcheck_gpg_status[i].result; > >> - sigc->key = xmemdupz(found, 16); > >> - found += 17; > >> - next = strchrnul(found, '\n'); > >> - sigc->signer = xmemdupz(found, next - found); > >> - break; > >> + if (sigc->result != 'U') { > > > > This could use a comment; we know now that only GOODSIG and BADSIG > > are followed by a signature, but someone looking at this code in the > > future will probably appreciate an explanation. > > Wouldn't it be even less magical if there was an explicit field in the > struct that says whether we need to read a sig from such a line? Yeah, that would be even better. > And furthermore, to use an enum instead of a char so that you can easily > spell out the details in the code? This also has the advantage that the > compiler can check that your 'switch'es cover all cases. > > That's of course assuming that I interpret the logic right; my current > understanding is that: > > * U means untrusted, which is bettern than B but worse than G; Yes, although I wonder if we should split TRUST_NEVER and TRUST_UNDEFINED (and maybe handle TRUST_MARGINAL as well) and print different messages in each case. > * GPG guarantees that the last line matching any of the patterns is the > one we care about, so we can blindly override one with the other The DETAILS file in GPG says: For each signature only one of the codes GOODSIG, BADSIG, EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted. so we should be OK here. -- 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