Thread (2 messages) flat view 2 messages, 2 authors, 2017-09-20

Re: [PATCH] commit: fix memory leak in `prepare_index()`

From: Jeff King <hidden>
Date: 2017-09-20 19:55:31

On Wed, Sep 20, 2017 at 09:47:23PM +0200, Martin Ågren wrote:
Release `pathspec` and the string list `partial`.
Makes sense.
When we clear the string list, make sure we do not free the `util`
pointers. That would result in double-freeing, since we set them up as
`item->util = item` in `list_paths()`.
Also makes sense (and is kind of weird; it looks like we're just using
it as a magic flag. But that's outside the scope of your patch).
Initialize the string list early, so that we can always release it. That
introduces some unnecessary overhead in various code paths, but means
there is one and only one way out of the function. If we ever accumulate
more things we need to free, it should be straightforward to do so.
The overhead is fine. It's just writing the struct entries, not
allocating anything. I wondered if the pathspec needed a similar
initialization (since you can't tell just from reading the context), but
no, it's initialized as the first thing in the function.
Signed-off-by: Martin Ågren <redacted>
---
 builtin/commit.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
Looks good to me. Thanks.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help