Re: [bug #33134] spurious error when stdout is already closed
On Thu, Apr 21, 2011 at 1:48 AM, Philip Guenther guent...@gmail.com wrote: [...] All quite true and admirably researched but this is not a standards-lawyering exercise, it's a software-engineering issue. I can't remember how many installers I've run which, when cancelled, finish by saying ERROR - the software is not installed!. And I always think That's not an error, I got what I asked for. The only error would be if the product ended up installed. Here we have the same thing. If the user has explicitly closed stdout before handing it to make, does s/he need to be alerted to that fact by make? And even if so, should it take the form of an ERROR message? I'd say no, because when the user gets exactly what s/he asked for that cannot be regarded as an error condition. If make feels the need to say Warning: unable to write to stdout because it's closed that would be reasonable, though a little fussy for my taste, but there's no error condition here. I feel like we're trapped in a classic Internet chest-thumping loop, but my day job requires that I break it. I'd say the bug report exists and this thread provides all the commentary one could wish, so I'm sure Paul will be in a position to make an informed decision when he gets to it, and let's leave it at that. David ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[bug #33134] spurious error when stdout is already closed
Follow-up Comment #1, bug #33134 (project make): Note that there is a thread on the bug-make mailing list discussing this in some detail: http://lists.gnu.org/archive/html/bug-make/2011-04/msg00077.html ___ 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 Thu, Apr 21, 2011 at 3:50 AM, David Boyce david.s.bo...@gmail.com wrote: All quite true and admirably researched but this is not a standards-lawyering exercise, it's a software-engineering issue. Why are you closing stdout instead of redirecting it to /dev/null? 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 Thu, Apr 21, 2011 at 11:29 AM, Philip Guenther guent...@gmail.com wrote: On Thu, Apr 21, 2011 at 3:50 AM, David Boyce david.s.bo...@gmail.com wrote: All quite true and admirably researched but this is not a standards-lawyering exercise, it's a software-engineering issue. Why are you closing stdout instead of redirecting it to /dev/null? While developing a patch for the .PARALLELSYNC feature being discussed in another thread (#33138), I tested a number of corner cases which included stdout being closed. That helped me find some bugs in my work but I also noticed that even unmodified make had this behavior, so I submitted a standalone patch to fix it. So - in this context at least - I'm not a user doing a dumb thing, I'm a developer doing a careful thing. In real life I've never had a valid reason to close stdout. David ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[bug #33134] spurious error when stdout is already closed
Follow-up Comment #2, bug #33134 (project make): My original one-line patch had two mistakes, which may be a record! In the course of work for enhancement #33138 I made a macro: #define STREAM_OK(strm) ((fcntl(fileno((strm)), F_GETFD) != -1) || (errno != EBADF)) whose logic would be much better than the original ftell(). Turns out that on many platforms ftell cannot be used on a non-seekable descriptor at all. My development is on Solaris where ftell works as a go/no-go indicator, but that isn't portable. And the EBADF test is required for reasons which are obvious and also discussed in the email thread. ___ 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
[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