Re: failure with HEAD: stdin seekable

[Eric Blake-1]

> 
> That sounds really tricky.  Much simpler is to say: if you care about
> the return value of a function, do the function before you call
> 'exit'.  Once you're exiting, don't issue any diagnostics: at that
> point your only goal is to exit cleanly.  This is the general strategy
> that coreutils uses.

Here's the patch I'm applying.  In non-interactive mode, the error
message actually improves - it now tracks the line number of the
m4exit that triggered the detection of the disk full write error.
But in interactive mode, stdout is unbuffered, hence the
write error is detected during the write rather than the fflush,
you've already lost the errno long before.  But I don't have any
problems with that - error messages are always best effort, as
long as the user can't ever get away with exiting without some
indication that an error was detected.  And even the m4exit
error message in non-interactive mode might fail to preserve
errno, if enough output was generated to reach an exact multiple
of the buffer size, making the fflush a no-op the way it is always
a no-op in interactive mode.

$ tests/m4 --interactive >/dev/full
hi m4exit(0)
m4: write error
$ tests/m4 --batch >/dev/full
hi m4exit(0)
m4:stdin:2: write error: No space left on device

I'm not sure this is worth backporting to the branch, particularly since
I'm not sure if we even need a 1.4.9 release yet.

2006-12-19  Eric Blake  <address@hidden>

        * modules/m4.h (m4_sysval_flush_func): Adjust prototype.
        * modules/m4.c (m4_sysval_flush): Add parameter, so that m4exit
        can track write errors without all other callers warning multiple
        times.
        (sysval_flush_helper): New function.
        (dumpdef, syscmd, errprint, m4exit): Adjust callers.
        * modules/gnu.c (esyscmd): Likewise.
        * tests/others.at (stdout closed, stdout full): Error message
        update.

Index: modules/gnu.c
===================================================================
RCS file: /sources/m4/m4/modules/gnu.c,v
retrieving revision 1.68
diff -u -r1.68 gnu.c
--- modules/gnu.c       14 Nov 2006 05:58:01 -0000      1.68
+++ modules/gnu.c       19 Dec 2006 17:14:24 -0000
@@ -539,7 +539,7 @@
          return;
        }
 
-      m4_sysval_flush (context);
+      m4_sysval_flush (context, false);
       errno = 0;
       pin = popen (M4ARG (1), "r");
       if (pin == NULL)
Index: modules/m4.c
===================================================================
RCS file: /sources/m4/m4/modules/m4.c,v
retrieving revision 1.95
diff -u -r1.95 m4.c
--- modules/m4.c        17 Dec 2006 04:21:16 -0000      1.95
+++ modules/m4.c        19 Dec 2006 17:14:24 -0000
@@ -53,7 +53,7 @@
 #define m4_make_temp           m4_LTX_m4_make_temp
 
 extern void m4_set_sysval    (int value);
-extern void m4_sysval_flush  (m4 *context);
+extern void m4_sysval_flush  (m4 *context, bool report);
 extern void m4_dump_symbols  (m4 *context, m4_dump_symbol_data *data, int
argc,
                              m4_symbol_value **argv, bool complain);
 extern const char *m4_expand_ranges (const char *s, m4_obstack *obs);
@@ -401,7 +401,7 @@
     }
 
   obstack_1grow (obs, '\0');
-  m4_sysval_flush (context);
+  m4_sysval_flush (context, false);
   fputs ((char *) obstack_finish (obs), stderr);
 }
 
@@ -481,17 +481,37 @@
   m4_sysval = value;
 }
 
+/* Flush a given output STREAM.  If REPORT, also print an error
+   message and clear the stream error bit.  */
+static void
+sysval_flush_helper (m4 *context, FILE *stream, bool report)
+{
+  if (fflush (stream) == EOF && report)
+    {
+      m4_error (context, 0, errno, _("write error"));
+      clearerr (stream);
+    }
+}
+
+/* Flush all user output streams, prior to doing something that can
+   could lose unflushed data or interleave debug and normal output
+   incorrectly.  If REPORT, then print an error message on failure and
+   clear the stream error bit; otherwise a subsequent ferror can track
+   that an error occurred.  */
 void
-m4_sysval_flush (m4 *context)
+m4_sysval_flush (m4 *context, bool report)
 {
   FILE *debug_file = m4_get_debug_file (context);
 
   if (debug_file != stdout)
-    fflush (stdout);
+    sysval_flush_helper (context, stdout, report);
   if (debug_file != stderr)
+    /* If we have problems with stderr, we can't really report that
+       problem to stderr.  The closeout module will ensure the exit
+       status reflects the problem, though.  */
     fflush (stderr);
   if (debug_file != NULL)
-    fflush (debug_file);
+    sysval_flush_helper (context, debug_file, report);
   /* POSIX requires that if m4 doesn't consume all input, but stdin is
      opened on a seekable file, that the file pointer be left at the
      next character on exit (but places no restrictions on the file
@@ -529,7 +549,7 @@
       m4_set_sysval (0);
       return;
     }
-  m4_sysval_flush (context);
+  m4_sysval_flush (context, false);
   m4_sysval = system (M4ARG (1));
   /* FIXME - determine if libtool works for OS/2, in which case the
      FUNC_SYSTEM_BROKEN section on the branch must be ported to work
@@ -803,7 +823,7 @@
   assert (obstack_object_size (obs) == 0);
   m4_dump_args (context, obs, argc, argv, " ", false);
   obstack_1grow (obs, '\0');
-  m4_sysval_flush (context);
+  m4_sysval_flush (context, false);
   fputs ((char *) obstack_finish (obs), stderr);
   fflush (stderr);
 }
@@ -839,7 +859,7 @@
   /* Change debug stream back to stderr, to force flushing debug
      stream and detect any errors.  */
   m4_debug_set_output (context, NULL);
-  m4_sysval_flush (context);
+  m4_sysval_flush (context, true);
 
   /* Check for saved error.  */
   if (exit_code == 0 && m4_get_exit_status (context) != 0)
Index: modules/m4.h
===================================================================
RCS file: /sources/m4/m4/modules/m4.h,v
retrieving revision 1.7
diff -u -r1.7 m4.h
--- modules/m4.h        21 Oct 2006 22:15:52 -0000      1.7
+++ modules/m4.h        19 Dec 2006 17:14:24 -0000
@@ -36,7 +36,7 @@
 
 /* Types used to cast imported symbols to, so we get type checking
    across the interface boundary.  */
-typedef void m4_sysval_flush_func (m4 *context);
+typedef void m4_sysval_flush_func (m4 *context, bool report);
 typedef void m4_set_sysval_func (int value);
 typedef void m4_dump_symbols_func (m4 *context, m4_dump_symbol_data *data,
                                   int argc, m4_symbol_value **argv,
Index: tests/others.at
===================================================================
RCS file: /sources/m4/m4/tests/others.at,v
retrieving revision 1.28
diff -u -r1.28 others.at
--- tests/others.at     15 Dec 2006 04:23:48 -0000      1.28
+++ tests/others.at     19 Dec 2006 17:14:24 -0000
@@ -540,7 +540,7 @@
 m4exit(`0')
 ]])
 AT_CHECK_M4([>&-], [1], [],
-[[m4: write error: Bad file descriptor
+[[m4:stdin:2: write error: Bad file descriptor
 ]], [in.m4])
 
 dnl preserve m4exit's failure value
@@ -548,7 +548,7 @@
 m4exit(`2')
 ]])
 AT_CHECK_M4([>&-], [2], [],
-[[m4: write error: Bad file descriptor
+[[m4:stdin:2: write error: Bad file descriptor
 ]], [in.m4])
 
 dnl trace file must not collide with closed stdout
@@ -647,14 +647,14 @@
 AT_DATA([in.m4], [[hello world m4exit(`0')
 ]])
 AT_CHECK_M4([in.m4 >/dev/full], [1], [],
-[[m4: write error: No space left on device
+[[m4:in.m4:1: write error: No space left on device
 ]])
 
 dnl preserve m4exit's failure value
 AT_DATA([in.m4], [[hello world m4exit(`2')
 ]])
 AT_CHECK_M4([in.m4 >/dev/full], [2], [],
-[[m4: write error: No space left on device
+[[m4:in.m4:1: write error: No space left on device
 ]])
 
 AT_CLEANUP

-- 
View this message in context: 
http://www.nabble.com/Re%3A-failure-with-HEAD%3A-stdin-seekable-tf2834388.html#a7950917
Sent from the Gnu - M4 - Patches mailing list archive at Nabble.com.