From: Omar Polo <op@omarpolo.com>
Subject: Re: memleak in commit_graph
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Sun, 04 Sep 2022 20:19:18 +0200

On 2022/09/04 16:55:39 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Sun, Sep 04, 2022 at 04:36:15PM +0200, Omar Polo wrote:
> > I'm chasing a memleak in gotwebd' blame page and landed here.  A dummy
> > commit graph loop seems to leak a bit so chances are that what I'm
> > seeing in gotwebd are due to the commit graph loop (via blame.c)
> 
> > regress is happy.  OK/further ideas for leaks in here?
> 
> These changes look right and regress is happy indeed. ok stsp@

and here's the diff to plug the missing leaks in here.

My previous diff free'd the commit that we didn't push into the
iter_list, but we need to eventually free also the item pushed into
the list!  (see second hunk.)

Additionally, we were leaking a bit of memory from add_node (it
returns a newly allocated struct) in packed_first_parent_traversal.

Both leaks spotted by valgrind.

With this valgrind is happy (for memleaks, it complains about some
possible off-by-one, will get thru those warnings later) and my repro
doesn't leak memory anymore.  I'm about to test this with gotwebd too.

diff /home/op/w/got
commit - 04666d1a54c25c8be7e39bc628b4a80f3376c127
path + /home/op/w/got
blob - ae6de06b406405893b822d52382ee6165539e3b3
file + lib/commit_graph.c
--- lib/commit_graph.c
+++ lib/commit_graph.c
@@ -232,6 +232,7 @@ packed_first_parent_traversal(int *ncommits_traversed,
 		err = add_node(&node, graph, &qid->id, repo);
 		if (err)
 			break;
+		free(node);
 	}
 
 	got_object_id_queue_free(&traversed_commits);
@@ -618,6 +619,7 @@ got_commit_graph_iter_next(struct got_object_id **id,
 
 	*id = &node->id;
 	TAILQ_REMOVE(&graph->iter_list, node, entry);
+	free(node);
 	return NULL;
 }