Re: [PATCH v2 2/3] builtin-status: submodule summary support

From: Ping Yin <pkufranky@gmail.com>
Date: Apr 12, 2008, 14:47 UTC
Message-ID: <46dff0320804120747j424d459oc0987beb27fce1c6@mail.gmail.com>
In-Reply-To: <7vtzi8owf9.fsf@gitster.siamese.dyndns.org>

On Sat, Apr 12, 2008 at 6:31 AM, Junio C Hamano <junio@pobox.com> wrote:

Show 16 quoted lines

>  > +static void wt_status_print_submodule_summary(struct wt_status *s)
>  > +{
>  > ...
>
> > +     memset(&sm_summary, 0, sizeof(sm_summary));
>  > +     sm_summary.argv = argv;
>  > +     sm_summary.env = env;
>  > +     sm_summary.git_cmd = 1;
>  > +     sm_summary.no_stdin = 1;
>  > +     fflush(s->fp);
>  > +     sm_summary.out = dup(fileno(s->fp));    /* run_command closes it */
>  > +     run_command(&sm_summary);
>
>  I recall we had some clean-up on how file descriptors are inherited and
>  duped around when run_command() runs a subprocess.  Hannes, is this dup()
>  consistent with how the things ought to be these days?

I think dup is needed, as commit c20181e3a3 says, run_command still will close passed in positive descripors.

    start_command(), if .in/.out > 0, closes file descriptors, not the callers

    Callers of start_command() can set the members .in and .out of struct
    child_process to a value > 0 to specify that this descriptor is used as
    the stdin or stdout of the child process.

    Previously, if start_command() was successful, this descriptor was closed
    upon return. Here we now make sure that the descriptor is also closed in
    case of failures. All callers are updated not to close the file descriptor
    themselves after start_command() was called.

-- 
Ping Yin