Am 08.06.2013 18:53, schrieb Felipe Contreras:
On Sat, Jun 8, 2013 at 10:56 AM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
Am 08.06.2013 16:04, schrieb Felipe Contreras:
On Sat, Jun 8, 2013 at 8:22 AM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
Am 08.06.2013 14:15, schrieb Felipe Contreras:
Why leave it out? If somebody makes the mistake of doing the above
sequence, would you prefer that we leak?
Leaking is better than silently cleaning up after a buggy caller because
it
still allows the underlying bug to be found.
No, it doesn't. The pointer is replaced and forever lost. How is
leaking memory helping anyone to find the bug?
Valgrind can tell you where leaked memory was allocated, but not if you free
it in the "wrong" place.
It is the correct place to free it. And nobody will ever find it with
valgrind, just like nobody has ever tracked down the hundreds of leaks
in Git that happen reliably 100% of the time, much less the ones that
happen rarely.
We could argue about freeing it here or adding a discard_index call
somewhere else (which seems more natural to me) once we had a call
sequence that actually causes such a leak. The test suite contains
none, so I'd say we need more tests first.
Lots of the existing leaks were not worth plugging because the process
would end right after freeing the memory. Leaving clean-up to the OS
was a conscious design choice, simplifying the code.
When such code is reused in a library or run multiple times while it was
originally meant to be run only a single time (like with cherry-pick
--stdin) the original assumption doesn't hold any more and we have a
problem.
Let's find and fix those leaks by freeing memory in the right places.
Freeing memory just in case in places where we can show that no leak is
triggered by our test suite doesn't help.
René
--
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