On Fri, Apr 05, 2013 at 11:57:19AM -0700, Junio C Hamano wrote:
> Petr Baudis <pa...@ucw.cz> writes:
> >> > -                        if (defined $opts{STDERR}) {
> >> > -                                close STDERR;
> >> > -                        }
> >> >                          if ($opts{STDERR}) {
> >> >                                  open (STDERR, '>&', $opts{STDERR})
> >
> >   I'm sorry, I don't follow. Doesn't this just break the STDERR option
> > altogether as we will try to dup2() over an already open file
> > descriptor? We do need to close STDERR if we are going to reopen it,
> > I think.
> When $opts{STDERR} is 2, what the three lines the proposed patch
> removes did is actively wrong, because you dup2 the fd you just
> closed.

Indeed, though $opts{STDERR} == 2 is something weird to do, it is a case
to consider.

> When $opts{STDERR} is 1, it seems to do the right thing with or
> without the "close STDERR" in front.  Isn't this because the usual
> "open($fd, <<<anything>>>) closes $fd as necessary" applies to this
> case as well?

I never actually tried that and was always happy to go with perldoc

        To (re)open "STDOUT" or "STDERR" as an in-memory file, close it first:
                   close STDOUT;
                   open(STDOUT, ">", \$variable)
                       or die "Can't open STDOUT: $!";

but my assumption that this generalizes to other kinds of open was
apparently invalid; an example further down the page proves me wrong
completely, moreover.

  The thing is, I was confused about dup2() all along as my old UNIX
masters taught me that I must close() the original descriptor first
and since that's what's commonly done anyway, I never thought to
double-check. Now I did and I learned something new, thanks!

I guess Acked-by: Petr Baudis <pa...@ucw.cz> then. :-)

                                Petr "Pasky" Baudis
        For every complex problem there is an answer that is clear,
        simple, and wrong.  -- H. L. Mencken
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to