Jim Meyering wrote:
> Thanks for writing all that. The code looks fine.
Glad to see that our disagreements have been reduced to the comments.
> Let's not use "signaled" here.
Yes, indeed this term is confusing in a paragraph dealing with signals.
> How about this in place of the above:
>
> /* Tell close_stdout whether to ignore an EPIPE error.
> The default (ignore = false) ensures that a close_stdout-induced write
> failure due to EPIPE evokes a diagnostic about the failure, along with
> a nonzero exit status. Use ignore = true to make close_stdout ignore
> any EPIPE error.
OK, I adopted the "diagnostic, along with a nonzero exit status" wording.
Still generally I prefer to say what the argument indicates, before stating
what is the default value. (It is less challenging for the reader to say
things one at a time.)
> Best not to use "signaled" here.
> Perhaps "diagnosed" instead.
Yup.
> > + The ignore = true setting is suitable for a scenario where you don't
> > know
>
> s/is/may be/
> Early reader termination may still deserve a diagnostic.
> Or it could be that POSIX requires the application to diagnose EPIPE,
> regardless ;-)
I disagree here. If early reader termination leads to a diagnostic in this
case, the diagnostic is timing dependent (depends which of the CPU cores
executing the reader process and the writer process is more loaded); this
is never what you want.
POSIX does not specify anthing here, because it essentially says that during
the operation specified by POSIX the SIGPIPE handler is set to SIG_DFL.
Btw, the wording "is suitable" is not as strong as "is mandatory": it is
merely an advice with explanations.
I committed this:
2008-10-05 Bruno Haible <[EMAIL PROTECTED]>
Jim Meyering <[EMAIL PROTECTED]>
Add an option for ignoring EPIPE during close_stdout.
* lib/closeout.h: Include <stdbool.h>.
(close_stdout_set_ignore_EPIPE): New declaration.
* lib/closeout.c: Include <stdbool.h>.
(ignore_EPIPE): New variable.
(close_stdout_set_ignore_EPIPE): New function.
(close_stdout): Ignore EPIPE error if ignore_EPIPE is set.
* lib/close-stream.c (close_stream): Mention the possible EPIPE
failure.
* modules/closeout (Depends-on): Add stdbool.
*** lib/close-stream.c.orig 2008-10-06 02:37:14.000000000 +0200
--- lib/close-stream.c 2008-10-06 01:50:55.000000000 +0200
***************
*** 1,6 ****
/* Close a stream, with nicer error checking than fclose's.
! Copyright (C) 1998, 1999, 2000, 2001, 2002, 2004, 2006, 2007 Free
Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
--- 1,6 ----
/* Close a stream, with nicer error checking than fclose's.
! Copyright (C) 1998, 1999, 2000, 2001, 2002, 2004, 2006, 2007, 2008 Free
Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
***************
*** 33,38 ****
--- 33,42 ----
otherwise. A failure might set errno to 0 if the error number
cannot be determined.
+ A failure with errno set to EPIPE may or may not indicate an error
+ situation worth signaling to the user. See the documentation of the
+ close_stdout_set_ignore_EPIPE function for details.
+
If a program writes *anything* to STREAM, that program should close
STREAM and make sure that it succeeds before exiting. Otherwise,
suppose that you go to the extreme of checking the return status
*** lib/closeout.c.orig 2008-10-06 02:37:14.000000000 +0200
--- lib/closeout.c 2008-10-06 02:36:36.000000000 +0200
***************
*** 1,6 ****
/* Close standard output and standard error, exiting with a diagnostic on
error.
! Copyright (C) 1998, 1999, 2000, 2001, 2002, 2004, 2006 Free
Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
--- 1,6 ----
/* Close standard output and standard error, exiting with a diagnostic on
error.
! Copyright (C) 1998, 1999, 2000, 2001, 2002, 2004, 2006, 2008 Free
Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
***************
*** 21,26 ****
--- 21,27 ----
#include "closeout.h"
#include <errno.h>
+ #include <stdbool.h>
#include <stdio.h>
#include <unistd.h>
***************
*** 42,47 ****
--- 43,85 ----
file_name = file;
}
+ static bool ignore_EPIPE /* = false */;
+
+ /* Specify the reaction to an EPIPE error during the closing of stdout:
+ - If ignore = true, it shall be ignored.
+ - If ignore = false, it shall evoke a diagnostic, along with a nonzero
+ exit status.
+ The default is ignore = false.
+
+ This setting matters only if the SIGPIPE signal is ignored (i.e. its
+ handler set to SIG_IGN) or blocked. Only particular programs need to
+ temporarily ignore SIGPIPE. If SIGPIPE is ignored or blocked because
+ it was ignored or blocked in the parent process when it created the
+ child process, it usually is a bug in the parent process: It is bad
+ practice to have SIGPIPE ignored or blocked while creating a child
+ process.
+
+ EPIPE occurs when writing to a pipe or socket that has no readers now,
+ when SIGPIPE is ignored or blocked.
+
+ The ignore = false setting is suitable for a scenario where it is normally
+ guaranteed that the pipe writer terminates before the pipe reader. In
+ this case, an EPIPE is an indication of a premature termination of the
+ pipe reader and should lead to a diagnostic and a nonzero exit status.
+
+ The ignore = true setting is suitable for a scenario where you don't know
+ ahead of time whether the pipe writer or the pipe reader will terminate
+ first. In this case, an EPIPE is an indication that the pipe writer can
+ stop doing useless write() calls; this is what close_stdout does anyway.
+ EPIPE is part of the normal pipe/socket shutdown protocol in this case,
+ and should not lead to a diagnostic message. */
+
+ void
+ close_stdout_set_ignore_EPIPE (bool ignore)
+ {
+ ignore_EPIPE = ignore;
+ }
+
/* Close standard output. On error, issue a diagnostic and _exit
with status 'exit_failure'.
***************
*** 68,74 ****
void
close_stdout (void)
{
! if (close_stream (stdout) != 0)
{
char const *write_error = _("write error");
if (file_name)
--- 106,113 ----
void
close_stdout (void)
{
! if (close_stream (stdout) != 0
! && !(ignore_EPIPE && errno == EPIPE))
{
char const *write_error = _("write error");
if (file_name)
*** lib/closeout.h.orig 2008-10-06 02:37:14.000000000 +0200
--- lib/closeout.h 2008-10-06 01:50:55.000000000 +0200
***************
*** 1,6 ****
/* Close standard output and standard error.
! Copyright (C) 1998, 2000, 2003, 2004, 2006 Free Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
--- 1,7 ----
/* Close standard output and standard error.
! Copyright (C) 1998, 2000, 2003, 2004, 2006, 2008 Free Software Foundation,
! Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
***************
*** 18,28 ****
--- 19,32 ----
#ifndef CLOSEOUT_H
# define CLOSEOUT_H 1
+ # include <stdbool.h>
+
# ifdef __cplusplus
extern "C" {
# endif
void close_stdout_set_file_name (const char *file);
+ void close_stdout_set_ignore_EPIPE (bool ignore);
void close_stdout (void);
# ifdef __cplusplus
*** modules/closeout.orig 2008-10-06 02:37:14.000000000 +0200
--- modules/closeout 2008-10-06 01:50:55.000000000 +0200
***************
*** 12,17 ****
--- 12,18 ----
error
quotearg
exitfail
+ stdbool
configure.ac:
gl_CLOSEOUT
