Re: [PATCH v2 1/2] Clean stale environment pointer in finish_command()

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> In start_command(), unset "env" fields are initialized via "env_array". In
> finish_command(), the "env_array" is cleared, therefore the "env" field
> will point to free()d data.
>
> However, start_command() will set "env" to env_array.argv only if "env"
> was unset to begin with, and if it was already set, the caller will need
> the original value. Therefore, we need to be very careful only to reset
> "env" in finish_command() when it has been initialized in start_command().

Hmph.  Does the same observation apply to cmd->argv that is
initialied to point to cmd->args.argv only when it is unset?

These managed argv/env arrays originate from c460c0ec (run-command:
store an optional argv_array, 2014-05-15), so I am CC'ing Peff.  I
think this change makes sense but I suspect we should do the same
for args.

>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  run-command.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/run-command.c b/run-command.c
> index 79a0a76..85578da 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -555,6 +555,9 @@ int finish_command(struct child_process *cmd)
>  {
>  	int ret = wait_or_whine(cmd->pid, cmd->argv[0]);
>  	argv_array_clear(&cmd->args);
> +	/* Avoid pointing to a stale environment */
> +	if (cmd->env == cmd->env_array.argv)
> +		cmd->env = NULL;
>  	argv_array_clear(&cmd->env_array);
>  	return ret;
>  }