Re: Re* [PATCH v5] describe: refresh the index when 'broken' flag is used

[Abhijeet Sonar <abhijeet.nkt@gmail.com>]

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 *);
>