[bug #33134] spurious error when stdout is already closed
URL: http://savannah.gnu.org/bugs/?33134 Summary: spurious error when stdout is already closed Project: make Submitted by: boyski Submitted on: Thu 21 Apr 2011 01:03:09 AM GMT Severity: 3 - Normal Item Group: Bug Status: None Privacy: Public Assigned to: None Open/Closed: Open Discussion Lock: Any Component Version: CVS Operating System: None Fixed Release: None Triage Status: None ___ Details: Ironically, make's attempt to be super-duper careful about catching write errors to stdout (in the close_stdout function) results in a spurious error message when the user has already closed stdout: % cat makefile .PHONY: all all:; date % make 1- make: write error Patch attached. ___ File Attachments: --- Date: Thu 21 Apr 2011 01:03:09 AM GMT Name: make-stdout-patch Size: 418B By: boyski http://savannah.gnu.org/bugs/download.php?file_id=23282 ___ Reply to this item at: http://savannah.gnu.org/bugs/?33134 ___ Message sent via/by Savannah http://savannah.gnu.org/ ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [bug #33134] spurious error when stdout is already closed
On Wed, Apr 20, 2011 at 6:03 PM, David Boyce invalid.nore...@gnu.org wrote: ... Ironically, make's attempt to be super-duper careful about catching write errors to stdout (in the close_stdout function) results in a spurious error message when the user has already closed stdout: % cat makefile .PHONY: all all:; date % make 1- make: write error Patch attached. Could you explain why you think that's spurious? Make wanted to write date to stdout and the write failed. Seems legit to me. (I'm not sure why it doesn't say make: write error: Bad file descriptor like it does if you change the command invoked to :, but that's a distinct issue.) Philip Guenther ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [bug #33134] spurious error when stdout is already closed
On Wed, Apr 20, 2011 at 9:58 PM, Philip Guenther guent...@gmail.com wrote:
Could you explain why you think that's spurious? Make wanted to write
date to stdout and the write failed. Seems legit to me.
That's actually not what generates the error message. I'm not sure why
it doesn't happen the way you say, but it doesn't. Did you look at the
patch? Here it is:
Index: main.c
===
RCS file: /sources/make/make/main.c,v
retrieving revision 1.246
diff -u -r1.246 main.c
--- main.c 29 Aug 2010 23:05:27 - 1.246
+++ main.c 21 Apr 2011 00:53:24 -
@@ -952,7 +959,8 @@
#endif
#ifdef HAVE_ATEXIT
- atexit (close_stdout);
+ if (ftell(stdout) != -1)
+atexit (close_stdout);
#endif
/* Needed for OS/2 */
Basically in a (sensible and nicely documented) attempt to detect all
errors, make does an explicit close of stdout just before exiting in
order to make one last check for failure modes. However, it makes the
mistake of assuming stdout was open to start with. So in fact what the
child process did with stdout has nothing to do with this bug. It's as
simple as closing an already-closed file descriptor and printing the
resulting error, and my patch is as simple as only close it if it's
open.
(I'm not sure why it doesn't say make: write error: Bad file
descriptor like it does if you change the command invoked to :, but
that's a distinct issue.)
I noticed this and found it interesting as well. It gets even stranger
too: looking at close_stdout() there are two possible messages:
if (prev_fail || fclose_fail)
{
if (fclose_fail)
error (NILF, _(write error: %s), strerror (errno));
else
error (NILF, _(write error));
exit (EXIT_FAILURE);
}
If you try the test case with -jN you get the first error but with -j
alone you get the second one.
David
___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
RE: [bug #33134] spurious error when stdout is already closed
maybe there's a better way of checking for closure than ftell
http://software.jessies.org/svn/salma-hayek/trunk/native/all/ruby-launcher/ruby-launcher.cpp
suggests:
// This might look obscure but the man page suggests that it's a
POSIX-compliant way
// of testing whether a file descriptor is open.
int rc = fcntl(targetFd, F_GETFL);
if (rc != -1) {
return;
}
if (errno != EBADF) {
Unlike make, that code only needs to worry about POSIX. I quote it mainly,
then, to suggest that doing this portably might prove surprisingly hard. It
would be portable and, if David's plausible explanation is right, more
intention-revealing, but more complicated and less efficient, to keep track of
whether stdout has ever been written to, and only close it then.
-Original Message-
From: bug-make-bounces+mdorey=bluearc@gnu.org
[mailto:bug-make-bounces+mdorey=bluearc@gnu.org] On Behalf Of David Boyce
Sent: Wednesday, April 20, 2011 21:32
To: Philip Guenther
Cc: bug-make
Subject: Re: [bug #33134] spurious error when stdout is already closed
On Thu, Apr 21, 2011 at 12:00 AM, Philip Guenther guent...@gmail.com wrote:
Why is that a mistake?
It appears you're saying that make should complain about failures to
write to stdout for reasons like EIO, ENOSPC, and EOVERFLOW, but
*not* for EBADF.
I think you're still not getting my point here. I do not believe this
has anything to do with *writes* at all, failed or otherwise. Make is
attempting to close something that's already closed and complaining
when it doesn't work. POSIX is quite clear that fclose on a closed
stream results in an error condition.
(Actually, your patch doesn't just ignore EBADF errors: it ignores
EPIPE errors, as the ftell() will fail on the pipe. Why is that a
good idea?)
You're right on this. An earlier version of my change, when it was
implemented within close_stdout(), looked something like
if (ftell(stdout) == -1 errno == EBADF) ...
but I lost the EBADF test when I redid it. That was a mistake, and
maybe there's a better way of checking for closure than ftell anyway,
but the basic point of not closing something unless it was open
remains.
David B
___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
