Re: [PATCH 03/15] read-cache: free cache entry in add_to_index in case of early return
- From
Stefan Beller <sbeller@google.com>
- Date
- Mar 21, 2015, 05:10 UTC
- Message-ID
- <CAGZ79kYVWUofm+Hc2tWv8vLBxc01HqCGM=efYyb0jdGB-64zBg@mail.gmail.com>
- In-Reply-To
- <xmqq1tkjc9g6.fsf@gitster.dls.corp.google.com>
On Fri, Mar 20, 2015 at 8:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> Stefan Beller <sbeller@google.com> writes: > >> This frees `ce` would be leaking in the error path. > > At this point ce is not yet added to the index, so it is clear it is > safe to free it---otherwise we will leak it. Good. > >> Additionally a free is moved towards the return. > > I am on the fence on this one between two schools and do not have a > strong preference. One school is to free as soon as you know you do > not need it, which is a valid stance to take. Another is, as you > did, not to care about the minimum necessary lifetime of the storage > and free them all at the end, which is also valid. Technically, the > former could be more performant while the latter is easier on the > eyes.
I only recall to have seen the latter school so far, which is why I made the change in the first place assuming the school of freeing ASAP has no strong supporters inside the git community.
I can resend the patch dropping the reordering, if you prefer.