From: Felipe Contreras Date: Mon, 03 Jun 2013 20:53:45 GMT Subject: Re: [PATCH v2 7/7] unpack-trees: free cache_entry array members for merges Message-ID: In-Reply-To: <7va9n7gkjq.fsf@alter.siamese.dyndns.org> On Mon, Jun 3, 2013 at 12:40 PM, Junio C Hamano wrote: > Felipe Contreras 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