On Fri, Mar 20, 2015 at 8:31 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> 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. -- 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