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?
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;
* 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
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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