From: Petr Baudis <pasky@ucw.cz>
To: Junio C Hamano <gitster@pobox.com>
Cc: Thomas Rast <trast@inf.ethz.ch>,
git@vger.kernel.org, Eric Wong <normalperson@yhbt.net>,
Marcin Owsiany <marcin@owsiany.pl>
Subject: Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing
Date: Sat, 6 Apr 2013 01:34:51 +0200 [thread overview]
Message-ID: <20130405233450.GA6137@machine.or.cz> (raw)
In-Reply-To: <7vsj34byb4.fsf@alter.siamese.dyndns.org>
On Fri, Apr 05, 2013 at 11:57:19AM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@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
maxim
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 <pasky@ucw.cz> then. :-)
--
Petr "Pasky" Baudis
For every complex problem there is an answer that is clear,
simple, and wrong. -- H. L. Mencken
next prev parent reply other threads:[~2013-04-06 16:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-03 22:26 [PATCH] perl: redirect stderr to /dev/null instead of closing Thomas Rast
2013-04-04 1:16 ` Eric Wong
2013-04-04 20:41 ` [PATCH v2 1/2] " Thomas Rast
2013-04-04 21:14 ` Eric Wong
2013-04-05 14:48 ` Petr Baudis
2013-04-05 18:57 ` Junio C Hamano
2013-04-05 23:34 ` Petr Baudis [this message]
2013-04-06 8:07 ` Thomas Rast
2013-04-06 10:34 ` Petr Baudis
2013-04-04 20:41 ` [PATCH v2 2/2] t9700: do not close STDERR Thomas Rast
2013-04-04 21:11 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130405233450.GA6137@machine.or.cz \
--to=pasky@ucw.cz \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=marcin@owsiany.pl \
--cc=normalperson@yhbt.net \
--cc=trast@inf.ethz.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).