Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: >> > A leak is better than a use after free, so >> > let's be extra careful here. Would it leave the index inconsistent? Or >> > perhaps freeing it has become safe in the meantime? >> > >> > @Junio: Do you remember the reason for the leaks in 0cf8581e330 >> > (checkout -m: recreate merge when checking out of unmerged index). >> >> Yes. >> >> In the very old days it was not allowed to free(3) contents of >> active_cache[] and this was an old brain fart that came out of >> inertia. We are manufacturing a brand new ce, only to feed it to >> checkout_entry() without even registering it to the active_cache[] >> array, and the ancient rule doesn't even apply to such a case. >> >> So I think it was safe to free(3) even back then. > > So this patch is good, then? Unless I from this year is failing to spot a breakage that will be caused in the codepath that I from year 2008 and René spotted, I think freeing ce after we are done updating the working tree file with it is safe. I'd need to find time to make sure, though. >> > And result_buf is still leaked here, right? >> >> Good spotting. It typically would make a larger leak than a single >> ce, I would suppose ;-) > > I saw you added this as a fixup! commit. If you don't mind, and if no > other changes are requested, would you mind rebase'ing this yourself? I think it would be better to leave the fix as a separate patch. It wasn't spotted by Coverity in the first place ;-)