diff mbox series

[v5,02/19] bundle.c: don't leak the "args" in the "struct child_process"

Message ID patch-v5-02.19-9eb758117dc-20230118T120334Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series leak fixes: various simple leak fixes | expand

Commit Message

Ævar Arnfjörð Bjarmason Jan. 18, 2023, 12:08 p.m. UTC
Fix a leak that's been here since 7366096de9d (bundle API: change
"flags" to be "extra_index_pack_args", 2021-09-05), if can't verify
the bundle we didn't call child_process_clear() to clear the "args".

But rather than doing that let's verify the bundle before we start
preparing the process we're going to spawn, if we get an error we
don't need to push anything to the "args".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 bundle.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Elijah Newren Jan. 26, 2023, 1:38 a.m. UTC | #1
On Wed, Jan 18, 2023 at 5:16 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> Fix a leak that's been here since 7366096de9d (bundle API: change
> "flags" to be "extra_index_pack_args", 2021-09-05), if can't verify
> the bundle we didn't call child_process_clear() to clear the "args".

That's really hard to parse.  Perhaps:

Fix a leak that's been here since 7366096de9d (bundle API: change
"flags" to be "extra_index_pack_args", 2021-09-05).  If we can't verify
the bundle, we didn't call child_process_clear() to clear the "args".

> But rather than doing that let's verify the bundle before we start
> preparing the process we're going to spawn, if we get an error we
> don't need to push anything to the "args".

Also hard to parse.  Perhaps:

But rather than adding an additional child_process_clear() call, let's
verify the bundle before we start
preparing the process we're going to spawn.  If we fail to verify, we
don't need to push anything to the child_process "args".


> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  bundle.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index 4ef7256aa11..9ebb10a8f72 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -627,6 +627,10 @@ int unbundle(struct repository *r, struct bundle_header *header,
>              enum verify_bundle_flags flags)
>  {
>         struct child_process ip = CHILD_PROCESS_INIT;
> +
> +       if (verify_bundle(r, header, flags))
> +               return -1;
> +
>         strvec_pushl(&ip.args, "index-pack", "--fix-thin", "--stdin", NULL);
>
>         /* If there is a filter, then we need to create the promisor pack. */
> @@ -638,8 +642,6 @@ int unbundle(struct repository *r, struct bundle_header *header,
>                 strvec_clear(extra_index_pack_args);
>         }
>
> -       if (verify_bundle(r, header, flags))
> -               return -1;
>         ip.in = bundle_fd;
>         ip.no_stdout = 1;
>         ip.git_cmd = 1;
> --
> 2.39.0.1225.g30a3d88132d
diff mbox series

Patch

diff --git a/bundle.c b/bundle.c
index 4ef7256aa11..9ebb10a8f72 100644
--- a/bundle.c
+++ b/bundle.c
@@ -627,6 +627,10 @@  int unbundle(struct repository *r, struct bundle_header *header,
 	     enum verify_bundle_flags flags)
 {
 	struct child_process ip = CHILD_PROCESS_INIT;
+
+	if (verify_bundle(r, header, flags))
+		return -1;
+
 	strvec_pushl(&ip.args, "index-pack", "--fix-thin", "--stdin", NULL);
 
 	/* If there is a filter, then we need to create the promisor pack. */
@@ -638,8 +642,6 @@  int unbundle(struct repository *r, struct bundle_header *header,
 		strvec_clear(extra_index_pack_args);
 	}
 
-	if (verify_bundle(r, header, flags))
-		return -1;
 	ip.in = bundle_fd;
 	ip.no_stdout = 1;
 	ip.git_cmd = 1;