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

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

Junio C Hamano <gitster@pobox.com> writes:

> (#leftoverbit)
>
> Outside the scope of this patch, I'd prefer to see somebody makes
> sure that it is truly equivalent to prepare a separate and new
> struct child_process for each run_command() call and to reuse the
> same struct child_process after calling child_process_clear() each
> time.  It is unclear if they are equivalent in general, even though
> in this particular case I think we should be OK.
>
> There _might_ be other things in the child_process structure that
> need to be reset to the initial state before it can be reused, but
> are not cleared by child_process_clear().  .git_cmd and other flags
> as well as in/out/err file descriptors do not seem to be cleared,
> and other callers of run_command() may even be depending on the
> current behaviour that they are kept.

Ahh, the reuse of the same struct came directly from Karthik's
review on the second iteration.  I guess Karthik volunteered himself
into this #leftoverbit task?  I am not convinced that

 (1) the selective clearing done by current child_process_clear() is
     the best thing we can do to make child_process reusable, and

 (2) among the current callers, there is nobody that depends on the
     state left by the previous use of child_process in another
     run_command() call that is left uncleared by child_process_clear().

If (1) is false, then reusing child_process structure is not quite
safe, and if (2) is false, updating child_process_clear() to really
clear everything will first need to adjust some callers.

Thanks.