Re: [PATCH v2 7/7] unpack-trees: free cache_entry array members for merges
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Jun 2, 2013, 17:59 UTC
- Message-ID
- <CAMP44s0Af90Sfi47_mxEGRFHAsAhaoO6T1jjbj7SptucNO-k0w@mail.gmail.com>
- In-Reply-To
- <51AB86BB.3080203@lsrfire.ath.cx>
On Sun, Jun 2, 2013 at 12:54 PM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
Show 19 quoted lines
> Am 02.06.2013 19:25, schrieb Felipe Contreras:
>>
>> On Sun, Jun 2, 2013 at 10:46 AM, René Scharfe
>> <rene.scharfe@lsrfire.ath.cx> wrote:
>>>
>>> + for (i = 0; i < n; i++) {
>>> + struct cache_entry *ce = src[i + o->merge];
>>> + if (ce != o->df_conflict_entry)
>>
>>
>> It's possible that ce is NULL, but you didn't add that check because
>> free(NULL) still works? Or because ce cannot be NULL?
>>
>> If it's the former, I think it's clearer if we check that ce is not
>> NULL either way.
>
>
> It is NULL if one tree misses an entry (e.g. a new or removed file). free
> handles NULL and we generally avoid duplicating its NULL-check.Yeah, but I can see somebody adding code inside that 'if' clause to print the cache entry, and see a crash only to wonder what's going on. And to save what? 5 characters?
-- Felipe Contreras