From: Ping Yin Date: Wed, 21 Nov 2007 02:38:33 GMT Subject: Re: [PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in Message-ID: <46dff0320711201838g5affba6bo21a8c837b0bef681@mail.gmail.com> In-Reply-To: <474308A5.8070301@viscovery.net> On Nov 21, 2007 12:17 AM, Johannes Sixt wrote: > Ping Yin schrieb: > > This patch disables the auto closing behavious of start_command > > and corrects all codes which depend on this kind of behaviour. > > I've thought about this a bit more, and I think that it is better to leave > this auto-closing behavior unchanged and change your usage of this feature, > like so: > > > +static void wt_status_print_submodule_summary(struct wt_status *s) > > +{ > > + struct child_process sm_summary; > > + memset(&sm_summary, 0, sizeof(sm_summary)); > > + ... > > + sm_summary.out = fileno(s->fp); > > fflush(s->fp); > sm_summary.out = dup(fileno(s->fp)); /* run_command closes it */ > > > + ... > > + run_command(&sm_summary); > > +} > > This way the change is more local without affecting well-tested other callers. > This way works, but it is a tricky one, not a natural or graceful one. > Furthermore, I don't think that it's correct to just set the .close_in or > .close_out flags. This will close the fd only in finish_command(), which can > be too late: Think again of a writable pipe end that remains open and keeps > the reader waiting for input that is not going to happen. This may happen. However, i have scanned all the git codes using the auto closing behaviour and i don't discover the problem you mentioned. So i think it deserves to correct the misbehaviour after carefully testing. And we can make a clarification for that if necessary. > > -- Hannes > > -- Ping Yin