patch, 2 partsrun-command: factor out child_process_clear()

> Call child_process_clear() when a child ends to release the memory
> allocated for its environment.  This is necessary because unlike all
> other users of start_command() we don't call finish_command(), which
> would have taken care of that for us.
> 
> This leak was introduced by f063d38b (daemon: use cld->env_array
> when re-spawning).

> Avoid duplication by moving the code to release allocated memory for
> arguments and environment to its own function, child_process_clear().
> Export it to provide a counterpart to child_process_init().
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---

>  Documentation/technical/api-run-command.txt |  7 +++++++
>  run-command.c                               | 15 +++++++++------
>  run-command.h                               |  1 +
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
> index a9fdb45..8bf3e37 100644
> --- a/Documentation/technical/api-run-command.txt
> +++ b/Documentation/technical/api-run-command.txt
> @@ -46,6 +46,13 @@ Functions
>  	The argument dir corresponds the member .dir. The argument env
>  	corresponds to the member .env.
>  
> +`child_process_clear`::
> +
> +	Release the memory associated with the struct child_process.
> +	Most users of the run-command API don't need to call this
> +	function explicitly because `start_command` invokes it on
> +	failure and `finish_command` calls it automatically already.
> +
>  The functions above do the following:
>  
>  . If a system call failed, errno is set and -1 is returned. A diagnostic
> diff --git a/run-command.c b/run-command.c
> index e17e456..13fa452 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -11,6 +11,12 @@ void child_process_init(struct child_process *child)
>  	argv_array_init(&child->env_array);
>  }
>  
> +void child_process_clear(struct child_process *child)
> +{
> +	argv_array_clear(&child->args);
> +	argv_array_clear(&child->env_array);
> +}
> +
>  struct child_to_clean {
>  	pid_t pid;
>  	struct child_to_clean *next;
> @@ -327,8 +333,7 @@ int start_command(struct child_process *cmd)
>  fail_pipe:
>  			error("cannot create %s pipe for %s: %s",
>  				str, cmd->argv[0], strerror(failed_errno));
> -			argv_array_clear(&cmd->args);
> -			argv_array_clear(&cmd->env_array);
> +			child_process_clear(cmd);
>  			errno = failed_errno;
>  			return -1;
>  		}
> @@ -513,8 +518,7 @@ fail_pipe:
>  			close_pair(fderr);
>  		else if (cmd->err)
>  			close(cmd->err);
> -		argv_array_clear(&cmd->args);
> -		argv_array_clear(&cmd->env_array);
> +		child_process_clear(cmd);
>  		errno = failed_errno;
>  		return -1;
>  	}
> @@ -540,8 +544,7 @@ fail_pipe:
>  int finish_command(struct child_process *cmd)
>  {
>  	int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
> -	argv_array_clear(&cmd->args);
> -	argv_array_clear(&cmd->env_array);
> +	child_process_clear(cmd);
>  	return ret;
>  }
>  
> diff --git a/run-command.h b/run-command.h
> index 5428b04..12bb26c 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -47,6 +47,7 @@ struct child_process {
>  
>  #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
>  void child_process_init(struct child_process *);
> +void child_process_clear(struct child_process *);
>  
>  int start_command(struct child_process *);
>  int finish_command(struct child_process *);

> René Scharfe <l.s.r@web.de> writes:
>
>> Avoid duplication by moving the code to release allocated memory for
>> arguments and environment to its own function, child_process_clear().
>> Export it to provide a counterpart to child_process_init().
>>
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>> ---
>
> Hmm, is this _deinit() Stefan added to his series recently?

> On Mon, Oct 26, 2015 at 11:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>> Avoid duplication by moving the code to release allocated memory for
>>> arguments and environment to its own function, child_process_clear().
>>> Export it to provide a counterpart to child_process_init().
>>>
>>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>>> ---
>>
>> Hmm, is this _deinit() Stefan added to his series recently?
>
> Yes. Although you (Junio) take credit for actually using it in these
> places, too. :)

> Am 26.10.2015 um 20:23 schrieb Stefan Beller:
>>
>> On Mon, Oct 26, 2015 at 11:43 AM, Junio C Hamano <gitster@pobox.com>
>> wrote:
>>>
>>> René Scharfe <l.s.r@web.de> writes:
>>>
>>>> Avoid duplication by moving the code to release allocated memory for
>>>> arguments and environment to its own function, child_process_clear().
>>>> Export it to provide a counterpart to child_process_init().
>>>>
>>>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>>>> ---
>>>
>>>
>>> Hmm, is this _deinit() Stefan added to his series recently?
>>
>>
>> Yes. Although you (Junio) take credit for actually using it in these
>> places, too. :)
>
>
> Oh, and it's already in next.  I still like _clear() better (as in
> {argv,object,ref,sha1}_array_clear(), or string_list_clear(), or ...) than
> _deinit(), which seems to have been introduced by vcs-svn and isn't a real
> word.  I'll rebase my patches on top of this series, but probably not before
> the weekend.
>