Re: add some fd tests to head

Eric Blake <ebb9 <at> byu.net> writes:

> 
> 2006-11-07  Eric Blake  <ebb9 <at> byu.net>
> 
> 	* tests/builtins.at (m4exit): New test; failed on cygwin before
> 	this patch.
> 	* m4/output.c (cleanup_tmpfile): Close files before removing
> 	directory.
> 	(make_room_for): Ensure that m4_error sees consistent state.

And this followup.

2006-11-07  Eric Blake  <[email protected]>

	* m4/output.c (cleanup_tmpfile, m4_insert_diversion_helper): Check
	for failure.

Index: m4/output.c
===================================================================
RCS file: /sources/m4/m4/m4/output.c,v
retrieving revision 1.36
diff -u -r1.36 output.c
--- m4/output.c	8 Nov 2006 04:26:53 -0000	1.36
+++ m4/output.c	8 Nov 2006 05:02:55 -0000
@@ -174,6 +174,7 @@
   m4_diversion *diversion;
   gl_list_iterator_t iter;
   const void *elt;
+  bool fail = false;
 
   if (diversion_table)
     {
@@ -182,14 +183,21 @@
       while (gl_list_iterator_next (&iter, &elt, NULL))
 	{
 	  diversion = (m4_diversion *) elt;
-	  if (!diversion->size && diversion->u.file)
-	    close_stream_temp (diversion->u.file);
+	  if (!diversion->size && diversion->u.file &&
+	      close_stream_temp (diversion->u.file) != 0)
+	    {
+	      error (0, errno,
+		     _("cannot clean temporary file for diversion"));
+	      fail = true;
+	    }
 	}
       gl_list_iterator_free (&iter);
     }
 
   /* Clean up the temporary directory.  */
   if (cleanup_temp_dir (output_temp_dir) != 0)
+    fail = true;
+  if (fail)
     _exit (exit_failure);
 }
 
@@ -664,8 +672,9 @@
       diversion->size = 0;
       diversion->used = 0;
     }
-  else if (diversion->u.file)
-    close_stream_temp (diversion->u.file);
+  else if (diversion->u.file && close_stream_temp (diversion->u.file) != 0)
+    m4_error (context, 0, errno,
+	      _("cannot clean temporary file for diversion"));
   gl_list_remove_node (diversion_table, node);
   diversion->u.next = free_list;
   free_list = diversion;


Today's patches ported to the branch as follows.

2006-11-07  Eric Blake  <[email protected]>

	* src/m4.h (output_exit): New prototype.
	* src/m4.c (main): Use it.
	* src/output.c (cleanup_tmpfile): Close files before removing
	directory.
	(insert_diversion): Check for failure.
	(output_exit): Avoid memory leak.
	* doc/m4.texinfo (Diversions): Test this bug.

Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.1.1.1.2.98
diff -u -r1.1.1.1.2.98 m4.texinfo
--- doc/m4.texinfo	7 Nov 2006 19:14:03 -0000	1.1.1.1.2.98
+++ doc/m4.texinfo	8 Nov 2006 05:01:34 -0000
@@ -3621,6 +3621,38 @@
 divert(`-1')undivert
 @end example
 
+@comment Another test of spilled diversions.
+
+@example
+divert(`-1')define(`f', `.')
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+define(`f', defn(`f')defn(`f'))
+divert`'dnl
+len(f)
+@result{}1048576
+divert(`1')
+f
+m4exit
+@end example
+
 @comment We also need to test allocation overflow.  On 32-bit
 @comment platforms, this should fail outright.  But on 64-bit platforms
 @comment with enough memory, the allocation might succeed (hopefully
Index: src/m4.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/m4.c,v
retrieving revision 1.1.1.1.2.37
diff -u -r1.1.1.1.2.37 m4.c
--- src/m4.c	1 Nov 2006 22:29:08 -0000	1.1.1.1.2.37
+++ src/m4.c	8 Nov 2006 05:01:34 -0000
@@ -576,5 +576,6 @@
       make_diversion (0);
       undivert_all ();
     }
+  output_exit ();
   exit (retcode);
 }
Index: src/m4.h
===================================================================
RCS file: /sources/m4/m4/src/m4.h,v
retrieving revision 1.1.1.1.2.33
diff -u -r1.1.1.1.2.33 m4.h
--- src/m4.h	1 Nov 2006 22:29:08 -0000	1.1.1.1.2.33
+++ src/m4.h	8 Nov 2006 05:01:34 -0000
@@ -311,6 +311,7 @@
 extern int output_current_line;
 
 void output_init (void);
+void output_exit (void);
 void shipout_text (struct obstack *, const char *, int);
 void make_diversion (int);
 void insert_diversion (int);
Index: src/output.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/output.c,v
retrieving revision 1.1.1.1.2.14
diff -u -r1.1.1.1.2.14 output.c
--- src/output.c	1 Nov 2006 22:29:08 -0000	1.1.1.1.2.14
+++ src/output.c	8 Nov 2006 05:01:34 -0000
@@ -106,13 +106,39 @@
   output_unused = 0;
 }
 
+void
+output_exit (void)
+{
+  free (diversion_table);
+  diversion_table = NULL;
+}
+
 /* Clean up any temporary directory.  Designed for use as an atexit
    handler, where it is not safe to call exit() recursively; so this
    calls _exit if a problem is encountered.  */
 static void
 cleanup_tmpfile (void)
 {
+  /* Close any open diversions.  */
+  int divnum;
+  struct diversion *diversion;
+  bool fail = false;
+
+  if (diversion_table)
+    for (divnum = 1; divnum < diversions; divnum++)
+      {
+	diversion = diversion_table + divnum;
+	if (diversion->file && close_stream_temp (diversion->file) != 0)
+	  {
+	    M4ERROR ((0, errno, "cannot clean temporary file for diversion"));
+	    fail = true;
+	  }
+      }
+
+  /* Clean up the temporary directory.  */
   if (cleanup_temp_dir (output_temp_dir) != 0)
+    fail = true;
+  if (fail)
     _exit (exit_failure);
 }
 
@@ -531,7 +557,8 @@
 
   if (diversion->file)
     {
-      close_stream_temp (diversion->file);
+      if (close_stream_temp (diversion->file) != 0)
+	M4ERROR ((0, errno, "cannot clean temporary file for diversion"));
       diversion->file = NULL;
     }
   else if (diversion->buffer)