Re: tee logs no output if stdout is closed

[Bruno Haible]

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  <address@hidden>
            Jim Meyering  <address@hidden>

        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