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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 3, 2013, 20:53 UTC
Message-ID: <CAMP44s2b7gNkiVddN8XVm_u4e4WkxWmvS0mB8XLRv-1tn2Qc9g@mail.gmail.com>
In-Reply-To: <7va9n7gkjq.fsf@alter.siamese.dyndns.org>

On Mon, Jun 3, 2013 at 12:40 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> I don't see that, and I don't like adding a check that I don't expect to be
>>> ever needed.
>>
>> It's called self-documenting code; by adding a check for the NULL
>> pointer, we are stating that ce can be NULL, if we don't do that,
>> people reading that code would need to figure that out themselves.
>
> People following the codepath to unpack_nondirectories() already
> have seen enough to know what src[] means and very well know what
> NULL in it means.  The only people possibly confused are those who
> do not know free(NULL) is safe, isn't it?

Wrong. I still do not know what src[] means, and I don't need to know, I can see from the code that the cached entries there leak.

> Honestly speaking, I do not want such people to be touching this
> part of the system.

So we should make it more obfuscated?

-- 
Felipe Contreras