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

On Sun, Jun 2, 2013 at 6:47 PM, René Scharfe
[off-list ref] wrote:

Am 03.06.2013 01:23, schrieb Felipe Contreras:

quoted

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.

quoted

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