Junio C Hamano <gitster@xxxxxxxxx> 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.
