threads / patch / 65035

patchpack-redundant: fix memory leak when open_pack_index() fails

Subject: [PATCH] pack-redundant: fix memory leak when open_pack_index() fails

## tl;dr

2 messages between Feb 21, 2026 and Feb 24, 2026. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Sahitya Chandra· Feb 21, 2026, 10:38 UTC · lore

In add_pack(), we allocate l.remaining_objects with llist_init() before calling open_pack_index(). If open_pack_index() fails we return NULL without freeing the allocated list, leaking the memory.

Fix by calling llist_free(l.remaining_objects) on the error path before returning.

Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
---
 builtin/pack-redundant.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Show changes to builtin/pack-redundant.c +3 −1

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index e4ecf774ca..86749bb7e7 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -546,8 +546,10 @@ static struct pack_list * add_pack(struct packed_git *p)
 	l.pack = p;
 	llist_init(&l.remaining_objects);
 
-	if (open_pack_index(p))
+	if (open_pack_index(p)) {
+		llist_free(l.remaining_objects);
 		return NULL;
+	}
 
 	base = p->index_data;
 	base += 256 * 4 + ((p->index_version < 2) ? 4 : 8);

-- 
2.43.0

Patrick Steinhardt· Feb 24, 2026, 10:14 UTC · re: Sahitya Chandra · lore

Re: [PATCH] pack-redundant: fix memory leak when open_pack_index() fails

On Sat, Feb 21, 2026 at 04:08:59PM +0530, Sahitya Chandra wrote:

> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index e4ecf774ca..86749bb7e7 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c

It's arguably not really worth it to work on git-pack-redundant(1) as it's deprecated and dies unless you pass "--i-still-use-this". But the fix is small enough, so it doesn't hurt much, either.

Show 9 quoted lines

> @@ -546,8 +546,10 @@ static struct pack_list * add_pack(struct packed_git *p)
>  	l.pack = p;
>  	llist_init(&l.remaining_objects);
>  
> -	if (open_pack_index(p))
> +	if (open_pack_index(p)) {
> +		llist_free(l.remaining_objects);
>  		return NULL;
> +	}

Right. The confusing part here is that `llist_init()` doesn't only initialize the data structure as its name might suggest, but it also ends up allocating memory. It would be great do adjust this interface to clarify, but that is certainly out of scope for this patch series.

By the way, can't we avoid the memory allocation altogether by reordering the code so that we try to open the pack before we allocate memory?

Thanks!

Patrick