Clean up leaks in commit-graph.c

While looking at the commit-graph code, I noticed some memory leaks. These can be found by running

valgrind --leak-check=full ./git commit-graph write --reachable

The impact of these leaks are small, as we never call write_commit_graph _reachable in a loop, but it is best to be diligent here.

While looking at memory consumption within write_commit_graph(), I noticed that we initialize our oid list with "object count / 4", which seems to be a huge over-count. I reduce this by a factor of eight.

I built off of ab/commit-graph-progress, because my patch involves lines close to those changes.

V2 includes feedback from V1 along with Martin's additional patches.

Thanks, -Stolee

Derrick Stolee (2):
commit-graph: clean up leaked memory during write
commit-graph: reduce initial oid allocation

Martin Ågren (1):
builtin/commit-graph.c: UNLEAK variables

 builtin/commit-graph.c | 11 ++++++-----
 commit-graph.c         | 16 ++++++++++------
 2 files changed, 16 insertions(+), 11 deletions(-)

base-commit: 6b89a34c89fc763292f06012318b852b74825619
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-42%2Fderrickstolee%2Fcommit-graph-leak-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-42/derrickstolee/commit-graph-leak-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/42

Range-diff vs v1:

1: 6906c25415 ! 1: ba65680b3d commit-graph: clean up leaked memory during write
@@ -7,17 +7,29 @@
leaked in write_commit_graph_reachable(). Clean these up so our memory checking is cleaner.

- Running 'valgrind --leak-check=full git commit-graph write
- --reachable' demonstrates these leaks and how they are fixed after
- this change.
+ Further, if we use a list of pack-files to find the commits, we
+ can leak the packed_git structs after scanning them for commits.

+ Running the following commands demonstrates the leak before and
+ the fix after:
+
+ * valgrind --leak-check=full ./git commit-graph write --reachable
+ * valgrind --leak-check=full ./git commit-graph write --stdin-packs
+
+ Signed-off-by: Martin Ågren <[email protected]>

diff --git a/commit-graph.c b/commit-graph.c
--- a/commit-graph.c
+++ b/commit-graph.c
@@
- string_list_init(&list, 1);
+ void write_commit_graph_reachable(const char *obj_dir, int append,
+ int report_progress)
+ {
+- struct string_list list;
++ struct string_list list = STRING_LIST_INIT_DUP;
+
+- string_list_init(&list, 1);
for_each_ref(add_ref_to_list, &list); write_commit_graph(obj_dir, NULL, &list, append, report_progress);
+
@@ -25,6 +37,14 @@
}

void write_commit_graph(const char *obj_dir,
+@@
+ die(_("error opening index for %s"), packname.buf);
+ for_each_object_in_pack(p, add_packed_commits, &oids, 0);
+ close_pack(p);
++ free(p);
+ }
+ stop_progress(&oids.progress);
+ strbuf_release(&packname);
@@
compute_generation_numbers(&commits, report_progress);

@@ -45,5 +65,8 @@
+ free(graph_name);
+ free(commits.list);
free(oids.list);
- oids.alloc = 0;
- oids.nr = 0;
+- oids.alloc = 0;
+- oids.nr = 0;
+ }
+
+ #define VERIFY_COMMIT_GRAPH_ERROR_HASH 2

 -:  ---------- > 2:  13032d8475 builtin/commit-graph.c: UNLEAK variables
 2:  e29a0eaf03 = 3:  1002fd34fc commit-graph: reduce initial oid allocation

gitgitgadget