Re: [PATCH v2 1/2] Clean stale environment pointer in finish_command()
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:54
Johannes Schindelin [off-list ref] 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.
quoted hunk
Signed-off-by: Johannes Schindelin <redacted> --- 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; }