{"thread":{"id":"33373","subject":"[PATCH] perl: redirect stderr to /dev/null instead of closing","startedAt":"2013-04-03T22:26:06Z","lastAt":"2013-04-06T10:34:26Z","messageCount":11,"participants":["Thomas Rast","Eric Wong","Jonathan Nieder","Junio C Hamano","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"213074","messageId":"f3d238a4c6cfbc6d68f2c4fa285aefa93acf4b7d.1365027616.git.trast@inf.ethz.ch","threadId":"33373","inReplyTo":null,"subject":"[PATCH] perl: redirect stderr to /dev/null instead of closing","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-03T22:26:06Z","receivedAt":"2013-04-03T22:26:06Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"On my system, t9100.1 triggers the following warning:\n\n  ==352== Syscall param write(buf) points to uninitialised byte(s)\n  ==352==    at 0x57119C0: __write_nocancel (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AC1D2: _IO_file_write@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AC0B1: new_do_write (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AD3B4: _IO_do_write@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AD6FE: _IO_file_overflow@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AE3D8: _IO_default_xsputn (in /lib64/libc-2.17.so)\n  ==352==    by 0x56ACAA2: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x5682133: buffered_vfprintf (in /lib64/libc-2.17.so)\n  ==352==    by 0x567CE9D: vfprintf (in /lib64/libc-2.17.so)\n  ==352==    by 0x5687096: fprintf (in /lib64/libc-2.17.so)\n  ==352==    by 0x4E7AC5: vreportf (usage.c:15)\n  ==352==    by 0x4E7B14: die_builtin (usage.c:38)\n\nThe actual complaint appears to be a bug in the underlying\nimplementation.  What's interesting here is that it is apparently\n_triggered_ by closing stderr, which results in (from strace)\n\n  write(2, \"fatal: Needed a single revision\\n\", 32) = -1 EBADF (Bad file descriptor)\n  write(2, \"\\0\", 1) = -1 EBADF (Bad file descriptor)\n\nClosing stderr is a bad idea anyway: there is a very real chance that\nwe print fatal error messages to some other file that just happens to\nbe opened on the now-free FD 2.  So let's not do that.\n\nSigned-off-by: Thomas Rast <trast@inf.ethz.ch>\n---\n\n\nThe commit message is intentionally overdramatic on the chance of\nprinting stuff to bad places.  The code is actually from way back in\n2006 (!).\n\nThe t9100 problem bisects to e3bd4dd (git-svn: don't create master if\nanother head exists, 2012-06-24), but that's just changing some\nverify_ref(), which asks to close stderr on the git-rev-parse process.\n\n\nI can easily reproduce the underlying issue with a small test: running\n\n  #include <stdio.h>\n\n  int main ()\n  {\n      \t  fprintf(stderr, \"%s%s\\n\", \"fatal: \", \"needed a single revision\");\n      \t  return 0;\n  }\n\nwith\n\n  valgrind --log-fd=3 ./die_test 3>&2 2>&-\n\nresults in pretty much the same warnings.  I fail to see a reason\nother than a glibc bug why\n\n  fprintf(stderr, \"%s%s\\n\", ...);\n\nshould attempt to write \"\\0\" -- all its inputs are C strings.  But\nmaybe I'm missing something?\n\n\n\n perl/Git.pm | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 96cac39..3b79a36 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -1495,6 +1495,9 @@ sub _command_common_pipe {\n \t\t\tif ($opts{STDERR}) {\n \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n \t\t\t\t\tor die \"dup failed: $!\";\n+\t\t\t} elsif (defined $opts{STDERR}) {\n+\t\t\t\topen (STDERR, '>', '/dev/null')\n+\t\t\t\t\tor die \"opening /dev/null failed: $!\";\n \t\t\t}\n \t\t\t_cmd_exec($self, $cmd, @args);\n \t\t}\n-- \n1.8.2.551.g91a1e48\n"},{"id":"213079","messageId":"20130404011653.GA28492@dcvr.yhbt.net","threadId":"33373","inReplyTo":"f3d238a4c6cfbc6d68f2c4fa285aefa93acf4b7d.1365027616.git.trast@inf.ethz.ch","subject":"Re: [PATCH] perl: redirect stderr to /dev/null instead of closing","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2013-04-04T01:16:53Z","receivedAt":"2013-04-04T01:16:53Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Thomas Rast <trast@inf.ethz.ch> wrote:\n> Closing stderr is a bad idea anyway: there is a very real chance that\n> we print fatal error messages to some other file that just happens to\n> be opened on the now-free FD 2.  So let's not do that.\n\n100% agreed.  FD 0, 1, and 2 should not be closed, way too much\npotential for triggering rare bugs and interop issues like these to be\nworth it.\n\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -1495,6 +1495,9 @@ sub _command_common_pipe {\n>  \t\t\tif ($opts{STDERR}) {\n>  \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n>  \t\t\t\t\tor die \"dup failed: $!\";\n> +\t\t\t} elsif (defined $opts{STDERR}) {\n> +\t\t\t\topen (STDERR, '>', '/dev/null')\n> +\t\t\t\t\tor die \"opening /dev/null failed: $!\";\n>  \t\t\t}\n>  \t\t\t_cmd_exec($self, $cmd, @args);\n>  \t\t}\n\nPerhaps we should also do the following:\n\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -1489,9 +1489,6 @@ sub _command_common_pipe {\n \t\tif (not defined $pid) {\n \t\t\tthrow Error::Simple(\"open failed: $!\");\n \t\t} elsif ($pid == 0) {\n-\t\t\tif (defined $opts{STDERR}) {\n-\t\t\t\tclose STDERR;\n-\t\t\t}\n \t\t\tif ($opts{STDERR}) {\n \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n \t\t\t\t\tor die \"dup failed: $!\";\n"},{"id":"213193","messageId":"801ebb2a75d7cddfeee70eb86e8854c78d22eb3e.1365107899.git.trast@inf.ethz.ch","threadId":"33373","inReplyTo":"20130404011653.GA28492@dcvr.yhbt.net","subject":"[PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-04T20:41:41Z","receivedAt":"2013-04-04T20:41:41Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"On my system, t9100.1 triggers the following warning:\n\n  ==352== Syscall param write(buf) points to uninitialised byte(s)\n  ==352==    at 0x57119C0: __write_nocancel (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AC1D2: _IO_file_write@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AC0B1: new_do_write (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AD3B4: _IO_do_write@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AD6FE: _IO_file_overflow@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x56AE3D8: _IO_default_xsputn (in /lib64/libc-2.17.so)\n  ==352==    by 0x56ACAA2: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)\n  ==352==    by 0x5682133: buffered_vfprintf (in /lib64/libc-2.17.so)\n  ==352==    by 0x567CE9D: vfprintf (in /lib64/libc-2.17.so)\n  ==352==    by 0x5687096: fprintf (in /lib64/libc-2.17.so)\n  ==352==    by 0x4E7AC5: vreportf (usage.c:15)\n  ==352==    by 0x4E7B14: die_builtin (usage.c:38)\n\nThe actual complaint appears to be a bug in the underlying\nimplementation.  What's interesting here is that it is apparently\n_triggered_ by closing stderr, which results in (from strace)\n\n  write(2, \"fatal: Needed a single revision\\n\", 32) = -1 EBADF (Bad file descriptor)\n  write(2, \"\\0\", 1) = -1 EBADF (Bad file descriptor)\n\nClosing stderr is a bad idea anyway: there is a very real chance that\nwe print fatal error messages to some other file that just happens to\nbe opened on the now-free FD 2.  So let's not do that.\n\nAs pointed out by Eric Wong (thanks), the initial close needs to go:\ndie() would again write nowhere if we close STDERR beforehand.\n\nSigned-off-by: Thomas Rast <trast@inf.ethz.ch>\n---\n\n> Perhaps we should also do the following:\n>\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -1489,9 +1489,6 @@ sub _command_common_pipe {\n>  \t\tif (not defined $pid) {\n>  \t\t\tthrow Error::Simple(\"open failed: $!\");\n>  \t\t} elsif ($pid == 0) {\n> -\t\t\tif (defined $opts{STDERR}) {\n> -\t\t\t\tclose STDERR;\n> -\t\t\t}\n>  \t\t\tif ($opts{STDERR}) {\n>  \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n>\t\t\t\t\tor die \"dup failed: $!\";\n\nIndeed.  Thanks for pointing that out.\n\n perl/Git.pm | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 96cac39..2cec8cf 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -1489,12 +1489,12 @@ sub _command_common_pipe {\n \t\tif (not defined $pid) {\n \t\t\tthrow Error::Simple(\"open failed: $!\");\n \t\t} elsif ($pid == 0) {\n-\t\t\tif (defined $opts{STDERR}) {\n-\t\t\t\tclose STDERR;\n-\t\t\t}\n \t\t\tif ($opts{STDERR}) {\n \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n \t\t\t\t\tor die \"dup failed: $!\";\n+\t\t\t} elsif (defined $opts{STDERR}) {\n+\t\t\t\topen (STDERR, '>', '/dev/null')\n+\t\t\t\t\tor die \"opening /dev/null failed: $!\";\n \t\t\t}\n \t\t\t_cmd_exec($self, $cmd, @args);\n \t\t}\n-- \n1.8.2.607.g19d29d3\n"},{"id":"213192","messageId":"3689abc8e1af4ddbbb7791dd6241996f86e4efa2.1365107899.git.trast@inf.ethz.ch","threadId":"33373","inReplyTo":"20130404011653.GA28492@dcvr.yhbt.net","subject":"[PATCH v2 2/2] t9700: do not close STDERR","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-04T20:41:42Z","receivedAt":"2013-04-04T20:41:42Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Much like the previous patch, this triggered an unrelated bug.\nClosing STDERR is not worth it anyway, as we risk writing die() and\nsuch to random files that happen to be subsequently opened on FD 2.\nDon't do it.\n\nSigned-off-by: Thomas Rast <trast@inf.ethz.ch>\n---\n t/t9700/test.pl | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9700/test.pl b/t/t9700/test.pl\nindex 0d4e366..1140767 100755\n--- a/t/t9700/test.pl\n+++ b/t/t9700/test.pl\n@@ -45,7 +45,8 @@ is($r->get_color(\"color.test.slot1\", \"red\"), $ansi_green, \"get_color\");\n # Failure cases for config:\n # Save and restore STDERR; we will probably extract this into a\n # \"dies_ok\" method and possibly move the STDERR handling to Git.pm.\n-open our $tmpstderr, \">&STDERR\" or die \"cannot save STDERR\"; close STDERR;\n+open our $tmpstderr, \">&STDERR\" or die \"cannot save STDERR\";\n+open STDERR, \">\", \"/dev/null\" or die \"cannot redirect STDERR to /dev/null\";\n is($r->config(\"test.dupstring\"), \"value2\", \"config: multivar\");\n eval { $r->config_bool(\"test.boolother\") };\n ok($@, \"config_bool: non-boolean values fail\");\n-- \n1.8.2.607.g19d29d3\n"},{"id":"213202","messageId":"20130404211114.GQ30308@google.com","threadId":"33373","inReplyTo":"3689abc8e1af4ddbbb7791dd6241996f86e4efa2.1365107899.git.trast@inf.ethz.ch","subject":"Re: [PATCH v2 2/2] t9700: do not close STDERR","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-04T21:11:15Z","receivedAt":"2013-04-04T21:11:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thomas Rast wrote:\n\n> --- a/t/t9700/test.pl\n> +++ b/t/t9700/test.pl\n> @@ -45,7 +45,8 @@ is($r->get_color(\"color.test.slot1\", \"red\"), $ansi_green, \"get_color\");\n>  # Failure cases for config:\n>  # Save and restore STDERR; we will probably extract this into a\n>  # \"dies_ok\" method and possibly move the STDERR handling to Git.pm.\n> -open our $tmpstderr, \">&STDERR\" or die \"cannot save STDERR\"; close STDERR;\n> +open our $tmpstderr, \">&STDERR\" or die \"cannot save STDERR\";\n> +open STDERR, \">\", \"/dev/null\" or die \"cannot redirect STDERR to /dev/null\";\n>  is($r->config(\"test.dupstring\"), \"value2\", \"config: multivar\");\n>  eval { $r->config_bool(\"test.boolother\") };\n>  ok($@, \"config_bool: non-boolean values fail\");\n>  open STDERR, \">&\", $tmpstderr or die \"cannot restore STDERR\";\n\nYeah, this makes sense.\n\nAt first I was confused: why not just let stderr go out to the console,\nwhere a person reading can see it?  But this test is meant to be run\nusing test_external_without_stderr, which redirects stderr to a file and\ndies if it ends up getting any content.\n\nperlfunc(1) documents the close-and-then-open trick for redirecting a\nfilehandle to an in-memory buffer.  Here a plain reopen works fine.\n\nSo for what it's worth\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"213203","messageId":"20130404211400.GA27728@dcvr.yhbt.net","threadId":"33373","inReplyTo":"801ebb2a75d7cddfeee70eb86e8854c78d22eb3e.1365107899.git.trast@inf.ethz.ch","subject":"Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2013-04-04T21:14:00Z","receivedAt":"2013-04-04T21:14:00Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Thomas Rast <trast@inf.ethz.ch> wrote:\n> As pointed out by Eric Wong (thanks), the initial close needs to go:\n> die() would again write nowhere if we close STDERR beforehand.\n> \n> Signed-off-by: Thomas Rast <trast@inf.ethz.ch>\n\nAcked-by: Eric Wong <normalperson@yhbt.net>\nThanks.\n"},{"id":"213298","messageId":"20130405144828.GX6137@machine.or.cz","threadId":"33373","inReplyTo":"801ebb2a75d7cddfeee70eb86e8854c78d22eb3e.1365107899.git.trast@inf.ethz.ch","subject":"Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2013-04-05T14:48:28Z","receivedAt":"2013-04-05T14:48:28Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"  Hi!\n\nOn Thu, Apr 04, 2013 at 10:41:41PM +0200, Thomas Rast wrote:\n> As pointed out by Eric Wong (thanks), the initial close needs to go:\n> die() would again write nowhere if we close STDERR beforehand.\n> \n> > Perhaps we should also do the following:\n> >\n> > --- a/perl/Git.pm\n> > +++ b/perl/Git.pm\n> > @@ -1489,9 +1489,6 @@ sub _command_common_pipe {\n> >  \t\tif (not defined $pid) {\n> >  \t\t\tthrow Error::Simple(\"open failed: $!\");\n> >  \t\t} elsif ($pid == 0) {\n> > -\t\t\tif (defined $opts{STDERR}) {\n> > -\t\t\t\tclose STDERR;\n> > -\t\t\t}\n> >  \t\t\tif ($opts{STDERR}) {\n> >  \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n> >\t\t\t\t\tor die \"dup failed: $!\";\n> \n> Indeed.  Thanks for pointing that out.\n\n  I'm sorry, I don't follow. Doesn't this just break the STDERR option\naltogether as we will try to dup2() over an already open file\ndescriptor? We do need to close STDERR if we are going to reopen it,\nI think.\n\n  Kind regards,\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"213289","messageId":"7vsj34byb4.fsf@alter.siamese.dyndns.org","threadId":"33373","inReplyTo":"20130405144828.GX6137@machine.or.cz","subject":"Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-05T18:57:19Z","receivedAt":"2013-04-05T18:57:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@ucw.cz> writes:\n\n>> >  \t\t} elsif ($pid == 0) {\n>> > -\t\t\tif (defined $opts{STDERR}) {\n>> > -\t\t\t\tclose STDERR;\n>> > -\t\t\t}\n>> >  \t\t\tif ($opts{STDERR}) {\n>> >  \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n>> >\t\t\t\t\tor die \"dup failed: $!\";\n>> \n>> Indeed.  Thanks for pointing that out.\n>\n>   I'm sorry, I don't follow. Doesn't this just break the STDERR option\n> altogether as we will try to dup2() over an already open file\n> descriptor? We do need to close STDERR if we are going to reopen it,\n> I think.\n\nWhen $opts{STDERR} is 2, what the three lines the proposed patch\nremoves did is actively wrong, because you dup2 the fd you just\nclosed.\n\nWhen $opts{STDERR} is 1, it seems to do the right thing with or\nwithout the \"close STDERR\" in front.  Isn't this because the usual\n\"open($fd, <<<anything>>>) closes $fd as necessary\" applies to this\ncase as well?\n\nSo, I am not sure what you are viewing as a problem.  Puzzled...\n"},{"id":"213347","messageId":"20130405233450.GA6137@machine.or.cz","threadId":"33373","inReplyTo":"7vsj34byb4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2013-04-05T23:34:51Z","receivedAt":"2013-04-05T23:34:51Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Fri, Apr 05, 2013 at 11:57:19AM -0700, Junio C Hamano wrote:\n> Petr Baudis <pasky@ucw.cz> writes:\n> >> > -\t\t\tif (defined $opts{STDERR}) {\n> >> > -\t\t\t\tclose STDERR;\n> >> > -\t\t\t}\n> >> >  \t\t\tif ($opts{STDERR}) {\n> >> >  \t\t\t\topen (STDERR, '>&', $opts{STDERR})\n> >\n> >   I'm sorry, I don't follow. Doesn't this just break the STDERR option\n> > altogether as we will try to dup2() over an already open file\n> > descriptor? We do need to close STDERR if we are going to reopen it,\n> > I think.\n> \n> When $opts{STDERR} is 2, what the three lines the proposed patch\n> removes did is actively wrong, because you dup2 the fd you just\n> closed.\n\nIndeed, though $opts{STDERR} == 2 is something weird to do, it is a case\nto consider.\n\n> When $opts{STDERR} is 1, it seems to do the right thing with or\n> without the \"close STDERR\" in front.  Isn't this because the usual\n> \"open($fd, <<<anything>>>) closes $fd as necessary\" applies to this\n> case as well?\n\nI never actually tried that and was always happy to go with perldoc\nmaxim\n\n\tTo (re)open \"STDOUT\" or \"STDERR\" as an in-memory file, close it first:\n\t           close STDOUT;\n\t           open(STDOUT, \">\", \\$variable)\n\t               or die \"Can't open STDOUT: $!\";\n\nbut my assumption that this generalizes to other kinds of open was\napparently invalid; an example further down the page proves me wrong\ncompletely, moreover.\n\n  The thing is, I was confused about dup2() all along as my old UNIX\nmasters taught me that I must close() the original descriptor first\nand since that's what's commonly done anyway, I never thought to\ndouble-check. Now I did and I learned something new, thanks!\n\nI guess Acked-by: Petr Baudis <pasky@ucw.cz> then. :-)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\n\tFor every complex problem there is an answer that is clear,\n\tsimple, and wrong.  -- H. L. Mencken\n"},{"id":"213327","messageId":"878v4wrsj7.fsf@linux-k42r.v.cablecom.net","threadId":"33373","inReplyTo":"20130405233450.GA6137@machine.or.cz","subject":"Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-06T08:07:40Z","receivedAt":"2013-04-06T08:07:40Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Petr Baudis <pasky@ucw.cz> writes:\n\n> On Fri, Apr 05, 2013 at 11:57:19AM -0700, Junio C Hamano wrote:\n>   The thing is, I was confused about dup2() all along as my old UNIX\n> masters taught me that I must close() the original descriptor first\n> and since that's what's commonly done anyway, I never thought to\n> double-check. Now I did and I learned something new, thanks!\n\nIndeed, that's the crucial point here.  dup2() is defined to close the\noriginal FD first if needed.\n\nIt's much saner this way for the case of stderr, as there is no time\nwhen we have no stderr available to report errors: the FD is replace\natomically from the POV of the program.\n\nThe manpage for dup2 does, however, say\n\n   If newfd was open, any errors  that  would  have  been  reported  at\n   close(2) time are lost.  A careful programmer will not use dup2() or\n   dup3() without closing newfd first.\n\nwhich is probably what you were referring to.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"213317","messageId":"20130406103426.GE6137@machine.or.cz","threadId":"33373","inReplyTo":"878v4wrsj7.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2013-04-06T10:34:26Z","receivedAt":"2013-04-06T10:34:26Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Sat, Apr 06, 2013 at 10:07:40AM +0200, Thomas Rast wrote:\n> The manpage for dup2 does, however, say\n> \n>    If newfd was open, any errors  that  would  have  been  reported  at\n>    close(2) time are lost.  A careful programmer will not use dup2() or\n>    dup3() without closing newfd first.\n> \n> which is probably what you were referring to.\n\nYes, that's probably one reason why I had this stuck in my mind (though,\nhow often does anyone bother to detect errors on close()...? ;-).\n\nFunnily enough, POSIX.2008 specifies that if closing newfd would fail,\ndup2() reports EIO and newfd is not closed, eliminating this problem.\n\nThe manpage does not cover this; well, that's fair enough as Linux just\ndoesn't care and never does that if I didn't miss anything in the code.\n\n-- \n\t\t\tPetr \"Pasky who might even send\n\t\t\t\ta patch, but the matter is\n\t\t\t\toh so obscure\" Baudis\n"}]}