Re: [PATCH] verify-tag: allow to verify signed blob objects

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Jun 15, 2016, 19:07 UTC
Message-ID: <9b700254-23b1-286c-70e6-68d484e9c4e8@drmicha.warpmail.net>
In-Reply-To: <xmqq8ty670v2.fsf@gitster.mtv.corp.google.com>

Junio C Hamano venit, vidit, dixit 15.06.2016 20:39:

Show 24 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> diff --git a/tag.c b/tag.c
>> index d1dcd18..d5f090b 100644
>> --- a/tag.c
>> +++ b/tag.c
>> @@ -39,7 +39,7 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
>>  	int ret;
>>  
>>  	type = sha1_object_info(sha1, NULL);
>> -	if (type != OBJ_TAG)
>> +	if ((type != OBJ_TAG) && ((type != OBJ_BLOB) || !(flags & GPG_VERIFY_BLOB)))
>>  		return error("%s: cannot verify a non-tag object of type %s.",
>>  				name_to_report ?
>>  				name_to_report :
> 
> The double negation is very hard to read.  I wonder
> 
> 	if ((type != OBJ_TAG) &&
>             !((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB)))
> 
> is easier to follow?  "It is not a tag object, and it's not like we
> were asked to verify blob and the user gave us a blob, either, so
> let's complain" is easier to follow, at least for me.

As a further exercise in boolean algebra, you can pull out the negation completely:

if (!( (type == OBJ_TAG) || ((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB)) ))

Show 7 quoted lines

> Or even
> 
> 	if ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB))
>                	"you told me to check blob but didn't give me one";
> 	} else if (type != OBJ_TAG)
> 		"you didn't give me a tag";
>

I just tried to stay as close to the original as possible, but I don't care either way. Your latter version is more strict and would require a slight documentation change, but would be fine, too.

Michael