public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pack-redundant: fix memory leak when open_pack_index() fails
@ 2026-02-21 10:38 Sahitya Chandra
  2026-02-24 10:14 ` Patrick Steinhardt
  0 siblings, 1 reply; 2+ messages in thread
From: Sahitya Chandra @ 2026-02-21 10:38 UTC (permalink / raw)
  To: git; +Cc: Sahitya Chandra, Junio C Hamano

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(-)

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] pack-redundant: fix memory leak when open_pack_index() fails
  2026-02-21 10:38 [PATCH] pack-redundant: fix memory leak when open_pack_index() fails Sahitya Chandra
@ 2026-02-24 10:14 ` Patrick Steinhardt
  0 siblings, 0 replies; 2+ messages in thread
From: Patrick Steinhardt @ 2026-02-24 10:14 UTC (permalink / raw)
  To: Sahitya Chandra; +Cc: git, Junio C Hamano

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.

> @@ -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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-02-24 10:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-21 10:38 [PATCH] pack-redundant: fix memory leak when open_pack_index() fails Sahitya Chandra
2026-02-24 10:14 ` Patrick Steinhardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox