Skip to contentSkip to conversation

fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

PATCH Superseded Lidong Yan via GitGitGadget 4 participants 7 messages May 8, 2025 – May 15, 2025

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

From: Lidong Yan <[email protected]>

In commit-graph.c line 1930, if open_pack_index failed, memory allocated in line 1925 by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

---
fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

In commit-graph.c line 1930, if open_pack_index failed, memory allocated in line 1925 by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1957%2Fbrandb97%2Ffix-commit-graph-leak-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1957/brandb97/fix-commit-graph-leak-v1
Pull-Request: https://github.com/git/git/pull/1957

 commit-graph.c | 2 ++
 1 file changed, 2 insertions(+)
commit-graph.c +2 -0
index 6394752b0b0..93d867770b0 100644
@@ -1929,6 +1929,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 		}
 		if (open_pack_index(p)) {
 			ret = error(_("error opening index for %s"), packname.buf);
+			close_pack(p);
+			free(p);
 			goto cleanup;
 		}
 		for_each_object_in_pack(p, add_packed_commits, ctx,

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 5 lines · Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>

In commit-graph.c line 1930, if open_pack_index failed, memory allocated in line 1925 by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

The same comments apply to this commit message as Junio has already mentioned in other commits. We don't typically point to exact line numbers, but rather mention for example the function name.

commit-graph.c static int fill_oids_from_packs(struct write_commit_graph_context *ctx, · line 1936

Quoted 13 lines
commit-graph.c +2 -0
index 6394752b0b0..93d867770b0 100644
@@ -1929,6 +1929,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 		}
 		if (open_pack_index(p)) {
 			ret = error(_("error opening index for %s"), packname.buf);
+			close_pack(p);
+			free(p);
 			goto cleanup;
 		}
 		for_each_object_in_pack(p, add_packed_commits, ctx,

The change itself looks correct to me. Thanks!

Patrick

#2 lore raw

Ok, I will replace line number to fill_oids_from_packs in next patch.

Quoted 31 lines

2025年5月9日 13:38,Patrick Steinhardt <[email protected]> 写道:

Quoted 5 lines · Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>

In commit-graph.c line 1930, if open_pack_index failed, memory allocated in line 1925 by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

The same comments apply to this commit message as Junio has already mentioned in other commits. We don't typically point to exact line numbers, but rather mention for example the function name.

commit-graph.c static int fill_oids_from_packs(struct write_commit_graph_context *ctx, · line 1936

Quoted 13 lines
commit-graph.c +2 -0
index 6394752b0b0..93d867770b0 100644
@@ -1929,6 +1929,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 }
 if (open_pack_index(p)) {
 ret = error(_("error opening index for %s"), packname.buf);
+ close_pack(p);
+ free(p);
 goto cleanup;
 }
 for_each_object_in_pack(p, add_packed_commits, ctx,

The change itself looks correct to me. Thanks!

Patrick

#3 lore raw

PATCH v2 1/1 fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

In reply to Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>

In commit-graph.c:fill_oids_from_packs, if open_pack_index failed, memory allocated and returned by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

---
fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

In commit-graph.c line 1930, if open_pack_index failed, memory allocated in line 1925 by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1957%2Fbrandb97%2Ffix-commit-graph-leak-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1957/brandb97/fix-commit-graph-leak-v2
Pull-Request: https://github.com/git/git/pull/1957

Range-diff vs v1:

1: 72402abe900 ! 1: 190961fe942 fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed
@@ Metadata
## Commit message ##
fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

- In commit-graph.c line 1930, if open_pack_index failed, memory allocated
- in line 1925 by add_packed_git will leak. Simply add close_pack and
- free(p) will solve this problem.
+ In commit-graph.c:fill_oids_from_packs, if open_pack_index failed,
+ memory allocated and returned by add_packed_git will leak. Simply
+ add close_pack and free(p) will solve this problem.

 commit-graph.c | 2 ++
 1 file changed, 2 insertions(+)
commit-graph.c +2 -0
index 6394752b0b0..93d867770b0 100644
@@ -1929,6 +1929,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 		}
 		if (open_pack_index(p)) {
 			ret = error(_("error opening index for %s"), packname.buf);
+			close_pack(p);
+			free(p);
 			goto cleanup;
 		}
 		for_each_object_in_pack(p, add_packed_commits, ctx,

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75

Signature · 1 line
gitgitgadget
#4 lore raw
Quoted 5 lines · Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>

In commit-graph.c:fill_oids_from_packs, if open_pack_index failed, memory allocated and returned by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

One more thing that I missed in the first review: the commit subject should mention the subsystem as prefix, not the specific function. So It should e.g. read:

commit-grap: fix memory leak when fill_oids_from_packs() fails

Patrick

#5 lore raw

PATCH v3 1/1 commit-grap: fix memory leak when `fill_oids_from_packs()` fails

In reply to Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>

In commit-graph.c:fill_oids_from_packs, if open_pack_index failed, memory allocated and returned by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

---
fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed

In commit-graph.c line 1930, if open_pack_index failed, memory allocated in line 1925 by add_packed_git will leak. Simply add close_pack and free(p) will solve this problem.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1957%2Fbrandb97%2Ffix-commit-graph-leak-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1957/brandb97/fix-commit-graph-leak-v3
Pull-Request: https://github.com/git/git/pull/1957

Range-diff vs v2:

1: 190961fe942 ! 1: e0dfe69f504 fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed
@@ Metadata
Author: Lidong Yan <[email protected]>

## Commit message ##
- fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed
+ commit-grap: fix memory leak when fill_oids_from_packs() fails

In commit-graph.c:fill_oids_from_packs, if open_pack_index failed, memory allocated and returned by add_packed_git will leak. Simply

 commit-graph.c | 2 ++
 1 file changed, 2 insertions(+)
commit-graph.c +2 -0
index 6394752b0b0..93d867770b0 100644
@@ -1929,6 +1929,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 		}
 		if (open_pack_index(p)) {
 			ret = error(_("error opening index for %s"), packname.buf);
+			close_pack(p);
+			free(p);
 			goto cleanup;
 		}
 		for_each_object_in_pack(p, add_packed_commits, ctx,

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75

Signature · 1 line
gitgitgadget
#6 lore raw

Re: [PATCH v3] commit-grap: fix memory leak when `fill_oids_from_packs()` fails

Quoted 3 lines · Lidong Yan via GitGitGadget

## Commit message ##
- fill_oids_from_packs: fix memory leak when fill_oids_from_packs failed
+ commit-grap: fix memory leak when fill_oids_from_packs() fails

I'll amend the title to add missing "h" after "grap" while queuing.

Thanks.