Re: [PATCH 1/5] submodule_move_head: fix leak of struct child_process

From: Brandon Williams <bmwill@google.com>
Date: May 1, 2017, 18:06 UTC
Message-ID: <20170501180610.GF39135@google.com>
In-Reply-To: <20170501180058.8063-2-sbeller@google.com>

On 05/01, Stefan Beller wrote:

> While fixing the leak of `cp`, reuse it instead of having to declare
> another struct child_process.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>

This shouldn't be needed as 'finish_command' does the cleanup for you by calling 'child_prcoess_clear()'.

Show 45 quoted lines

> ---
>  submodule.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index d3299e29c0..cd098cf12b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1466,17 +1466,19 @@ int submodule_move_head(const char *path,
>  		goto out;
>  	}
>  
> +	child_process_clear(&cp);
> +
>  	if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
>  		if (new) {
> -			struct child_process cp1 = CHILD_PROCESS_INIT;
> +			child_process_init(&cp);
>  			/* also set the HEAD accordingly */
> -			cp1.git_cmd = 1;
> -			cp1.no_stdin = 1;
> -			cp1.dir = path;
> +			cp.git_cmd = 1;
> +			cp.no_stdin = 1;
> +			cp.dir = path;
>  
> -			argv_array_pushl(&cp1.args, "update-ref", "HEAD", new, NULL);
> +			argv_array_pushl(&cp.args, "update-ref", "HEAD", new, NULL);
>  
> -			if (run_command(&cp1)) {
> +			if (run_command(&cp)) {
>  				ret = -1;
>  				goto out;
>  			}
> @@ -1492,6 +1494,7 @@ int submodule_move_head(const char *path,
>  		}
>  	}
>  out:
> +	child_process_clear(&cp);
>  	return ret;
>  }
>  
> -- 
> 2.13.0.rc1.1.gbc33f0f778
>

-- 
Brandon Williams