diff mbox series

[1/3] dir: fix leak of parent_hashmap and recursive_hashmap

Message ID 932741d7598ca2934dbca40f715ba2d3819fcc51.1597561152.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Clean up some memory leaks in and around dir.c | expand

Commit Message

Johannes Schindelin via GitGitGadget Aug. 16, 2020, 6:59 a.m. UTC 
From: Elijah Newren <newren@gmail.com>

Commit 96cc8ab531 ("sparse-checkout: use hashmaps for cone patterns",
2019-11-21) added a parent_hashmap and recursive_hashmap to each struct
pattern_list and initialized these in add_patterns_from_buffer() but did
not make sure to add necessary code to clear_pattern_list() to free
these new structures.  Call hashmap_free_() on each to plug this memory
leak.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 dir.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jeff King Aug. 16, 2020, 8:43 a.m. UTC | #1 
On Sun, Aug 16, 2020 at 06:59:09AM +0000, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
> 
> Commit 96cc8ab531 ("sparse-checkout: use hashmaps for cone patterns",
> 2019-11-21) added a parent_hashmap and recursive_hashmap to each struct
> pattern_list and initialized these in add_patterns_from_buffer() but did
> not make sure to add necessary code to clear_pattern_list() to free
> these new structures.  Call hashmap_free_() on each to plug this memory
> leak.

Beat you to it. :)

See: https://lore.kernel.org/git/20200814111049.GA4101811@coredump.intra.peff.net/

-Peff
Elijah Newren Aug. 17, 2020, 4:57 p.m. UTC | #2 
On Sun, Aug 16, 2020 at 1:43 AM Jeff King <peff@peff.net> wrote:
>
> On Sun, Aug 16, 2020 at 06:59:09AM +0000, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Commit 96cc8ab531 ("sparse-checkout: use hashmaps for cone patterns",
> > 2019-11-21) added a parent_hashmap and recursive_hashmap to each struct
> > pattern_list and initialized these in add_patterns_from_buffer() but did
> > not make sure to add necessary code to clear_pattern_list() to free
> > these new structures.  Call hashmap_free_() on each to plug this memory
> > leak.
>
> Beat you to it. :)
>
> See: https://lore.kernel.org/git/20200814111049.GA4101811@coredump.intra.peff.net/

Doh!  However, you do need to take care to free the hash table in
addition to the hash entries, or you'll still have a leak (I actually
made the same mistake)...  I'll add those comments on your patch,
though.
diff mbox series

Patch

diff --git a/dir.c b/dir.c
index fe64be30ed..08df469bf7 100644
--- a/dir.c
+++ b/dir.c
@@ -916,6 +916,10 @@  void clear_pattern_list(struct pattern_list *pl)
 		free(pl->patterns[i]);
 	free(pl->patterns);
 	free(pl->filebuf);
+	hashmap_free_(&pl->parent_hashmap,
+		      offsetof(struct pattern_entry, ent));
+	hashmap_free_(&pl->recursive_hashmap,
+		      offsetof(struct pattern_entry, ent));
 
 	memset(pl, 0, sizeof(*pl));
 }