Junio C Hamano <gitster@xxxxxxxxx> writes: > 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 > Hehe. I'll take it up! > (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. > I think it would be best to write some unit tests to capture the current behavior and based on the findings and as you suggested, we can decide the path forward. Karthik
Attachment:
signature.asc
Description: PGP signature
