Re: [PATCH 4/6] Convert struct name_entry to use struct object_id.

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 20, 2016, 00:17 UTC
Message-ID: <xmqqk2jtxhf3.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160419231617.GA572694@vauxhall.crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 17 quoted lines

> On Tue, Apr 19, 2016 at 04:02:22PM -0700, Junio C Hamano wrote:
>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> 
>> > @@ -314,7 +314,7 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
>> >  	}
>> >  
>> >  	if (same_entry(entry+0, entry+1)) {
>> > -		if (entry[2].sha1 && !S_ISDIR(entry[2].mode)) {
>> > +		if (entry[2].oid->hash && !S_ISDIR(entry[2].mode)) {
>> 
>> Thanks for a warning in the cover letter.
>> 
>> "if (entry[2].oid && !S_ISDIR(entry[2].mode)" would be a faithful
>> conversion, wouldn't it?
>
> Yes, I think that would be a better conversion.  I'll reroll after
> waiting for further comments.

Thanks. A simple general rule to follow is that anything that assumed entry.sha1 is non-NULL should become entry.oid->hash, while anyting that checked if entry.sha1 is NULL or not should become a check on entry.oid, I think. Even though offsetof(oid.hash) might happen to be zero, compiler writers are crazy and "optimize" the above entry[2].oid->hash saying "you are dereerencing into the hash field without first checking if oid pointer is NULL, so we'd assume that you know oid is not NULL here--which makes this always true so we won't even do any check and only check entry[2].mode"..