From: Derrick Stolee <stolee@gmail.com>
Date: Mon, 27 Aug 2018 12:31:46 GMT
Subject: Re: [PATCH 3/9] convert "oidcmp() == 0" to oideq()
Message-ID: <62172db4-15b2-eac3-8591-246300db3948@gmail.com>
In-Reply-To: <20180825083602.GA2251@sigill.intra.peff.net>

On 8/25/2018 4:36 AM, Jeff King wrote:
> On Sat, Aug 25, 2018 at 04:07:15AM -0400, Jeff King wrote:
>
>> diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
>> index 5869979be7..548c02336d 100644
>> --- a/contrib/coccinelle/object_id.cocci
>> +++ b/contrib/coccinelle/object_id.cocci
>> @@ -108,3 +108,9 @@ expression E1, E2;
>>   @@
>>   - hashcpy(E1.hash, E2->hash)
>>   + oidcpy(&E1, E2)

Is this change intended? It doesn't seem to match the intention of the 
rest of the patch.

>> +
>> +@@
>> +expression E1, E2;
>> +@@
>> +- oidcmp(E1, E2) == 0
>> ++ oideq(E1, E2)
>>
>> [...]
>>
>> diff --git a/cache.h b/cache.h
>> index f6d227fac7..d090f71706 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -1100,7 +1100,7 @@ static inline int is_empty_blob_sha1(const unsigned char *sha1)
>>   
>>   static inline int is_empty_blob_oid(const struct object_id *oid)
>>   {
>> -	return !oidcmp(oid, the_hash_algo->empty_blob);
>> +	return oideq(oid, the_hash_algo->empty_blob);
>>   }
> By the way, one interesting thing I noticed is that coccinelle generates
> the hunk for cache.h for _every_ file that includes it, and the
> resulting patch is annoying to apply. I think this is related to the
> discussion we were having in:
>
>    https://public-inbox.org/git/20180802115522.16107-1-szeder.dev@gmail.com/
>
> Namely that we might do better to invoke one big spatch (and let it
> parallelize internally) instead of one perfile. Presumably that would
> also avoid looking at the headers repeatedly (which would be both faster
> and produce better output).
>
> I'm not planning to pursue that immediately, so just food for thought at
> this point.

The more we use Coccinelle, the more we will learn how to improve the 
workflow.

Thanks,

-Stolee