Re: [PATCH v2 7/7] unpack-trees: free cache_entry array members for merges

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 2, 2013 at 6:47 PM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
> Am 03.06.2013 01:23, schrieb Felipe Contreras:
>
>> I didn't say we should do 'if (ce) free(ce);' instead of 'free(ce);' I
>> said we should do 'if (cd && ce != o->df_conflict_entry)' instead of
>> 'if (ce != o->df_conflict_entry)'.
>
>
> I did assume you meant the latter.
>
>
>> There's no reason not to.
>
>
> Only the minor ones already mentioned: More text,

Five characters.

> one more branch in object
> code,

Which might actually be more optimal.

> no benefit except for some hypothetical future case that's caught by
> the test suite anyway -- or by code review.

That's not the benefit, the benefit is that the code is clearer.

> I wonder if we already reached the point where we spent more time discussing
> this change than the time needed by the envisioned developer to find and fix
> the NULL check that suddenly became necessary. :)

Maybe, but if what you want is to avoid the discussion, you could just
add the extra five characters and be done with it.

-- 
Felipe Contreras
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]