On 26/06/24 21:47, Junio C Hamano wrote:
> Or alternatively, we could do this to ensure that the child_process
> structure is always reusable.
>
> run-command.c | 1 +
> run-command.h | 6 +++++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git c/run-command.c w/run-command.c
> index 6ac1d14516..aba250fbe1 100644
> --- c/run-command.c
> +++ w/run-command.c
> @@ -26,6 +26,7 @@ void child_process_clear(struct child_process *child)
> {
> strvec_clear(&child->args);
> strvec_clear(&child->env);
> + child_process_init(child);
> }
To me, this looks much better. child_process_clear's name already
suggests that is sort of like a destructor, so it makes sense to
re-initialize everything here. I even wonder why it was not that way to
begin with. I suppose no callers are assuming that it only clears args
and env though?
> struct child_to_clean {
> diff --git c/run-command.h w/run-command.h
> index 55f6631a2a..6e203c22f6 100644
> --- c/run-command.h
> +++ w/run-command.h
> @@ -204,7 +204,8 @@ int start_command(struct child_process *);
>
> /**
> * Wait for the completion of a sub-process that was started with
> - * start_command().
> + * start_command(). The child_process structure is cleared and
> + * reinitialized.
> */
> int finish_command(struct child_process *);
>
> @@ -214,6 +215,9 @@ int finish_command_in_signal(struct child_process *);
> * A convenience function that encapsulates a sequence of
> * start_command() followed by finish_command(). Takes a pointer
> * to a `struct child_process` that specifies the details.
> + * The child_process structure is cleared and reinitialized,
> + * even when the command fails to start or an error is detected
> + * in finish_command().
> */
> int run_command(struct child_process *);
>
