Re: branch-1_4 allocation overflow

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

> 
> $ build/src/m4
> include(examples/forloop.m4)dnl
> forloop(i,1,2200,`divert(i)divnum
> ')
> build/src/m4:stdin:2: cannot create temporary file for diversion: Too many 
open 
> files
> Aborted (core dumped)

For that matter, the arbitrary limit of EMFILE can be worked around on the 
branch:

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

	* src/output.c (cleanup_tmpfile): Avoid double error message when
	umask is prohibitive.
	(m4_tmpname, m4_tmpopen, m4_tmpclose, m4_tmpremove): New
	functions.
	(m4_tmpfile): Add parameter, move cloexec action here.
	(make_room_for): Adjust caller.  Don't keep too many files open.
	(insert_diversion_helper): Unlink emptied temp files.
	(make_diversion): Don't keep too many files open.
	* doc/m4.texinfo (Diversions): Tweak wording, now that open file
	descriptors are no longer a limiting factor.
	* NEWS: Document this change.

Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.1.1.1.2.81
diff -u -r1.1.1.1.2.81 NEWS
--- NEWS	13 Nov 2006 19:10:56 -0000	1.1.1.1.2.81
+++ NEWS	13 Nov 2006 23:19:20 -0000
@@ -8,7 +8,8 @@
   cause a core dump when handed extra large values.  Also, `divert' now
   uses memory proportional to the number of diversions in use, rather than
   to the maximum diversion number encountered, so that large diversion
-  numbers are less likely to exhaust system memory.
+  numbers are less likely to exhaust system memory; and is no longer
+  limited by the maximum number of file descriptors.
 * The `--help' and `--version' command line options now consistently
   override all earlier options.  For example, `m4 --debugfile=trace
   --help' now no longer accidentally creates an empty file `trace'.
Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.1.1.1.2.101
diff -u -r1.1.1.1.2.101 m4.texinfo
--- doc/m4.texinfo	13 Nov 2006 19:10:56 -0000	1.1.1.1.2.101
+++ doc/m4.texinfo	13 Nov 2006 23:19:20 -0000
@@ -3584,8 +3584,8 @@
 diversion still in memory, freeing this memory for other diversions.
 When creating the temporary file, @code{m4} honors the value of the
 environment variable @env{TMPDIR}, and falls back to @file{/tmp}.
-So, it is theoretically possible that the number of diversions be
-limited by the number of available file descriptors.
+So, it is theoretically possible that the number and aggregate size of
+diversions is limited only by available disk space.
 
 @ignore
 @comment We need to test spilled diversions, but don't need to expose
Index: src/output.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/output.c,v
retrieving revision 1.1.1.1.2.16
diff -u -r1.1.1.1.2.16 output.c
--- src/output.c	13 Nov 2006 19:10:57 -0000	1.1.1.1.2.16
+++ src/output.c	13 Nov 2006 23:19:20 -0000
@@ -21,6 +21,7 @@
 
 #include "m4.h"
 
+#include <limits.h>
 #include <sys/stat.h>
 
 #include "gl_avltree_oset.h"
@@ -47,8 +48,9 @@
 /* When part of diversion_table, each struct m4_diversion either
    represents an open file (zero size, non-NULL u.file), an in-memory
    buffer (non-zero size, non-NULL u.buffer), or an unused placeholder
-   diversion (zero size, u is NULL).  When not part of
-   diversion_table, u.next is a pointer to the free_list chain.  */
+   diversion (zero size, u is NULL, non-zero used indicates that a
+   file has been created).  When not part of diversion_table, u.next
+   is a pointer to the free_list chain.  */
 
 typedef struct m4_diversion m4_diversion;
 
@@ -180,16 +182,32 @@
     _exit (exit_failure);
 }
 
-/* Create a temporary file open for reading and writing in a secure
-   temp directory.  The file will be automatically closed and deleted
-   on a fatal signal.  When done with the file, close it with
-   close_stream_temp.  Exits on failure, so the return value is always
-   an open file.  */
+/* Convert DIVNUM into a temporary file name for use in m4_tmp*.  */
+static const char *
+m4_tmpname (int divnum)
+{
+  static char *buffer;
+  static char *tail;
+  if (buffer == NULL)
+    {
+      tail = xasprintf ("%s/m4-%d", output_temp_dir->dir_name, INT_MAX);
+      buffer = obstack_copy0 (&diversion_storage, tail, strlen (tail));
+      tail = strrchr (buffer, '-') + 1;
+    }
+  sprintf (tail, "%d", divnum);
+  return buffer;
+}
+
+/* Create a temporary file for diversion DIVNUM open for reading and
+   writing in a secure temp directory.  The file will be automatically
+   closed and deleted on a fatal signal.  The file can be closed and
+   reopened with m4_tmpclose and m4_tmpopen; when finally done with
+   the file, close it with m4_tmpremove.  Exits on failure, so the
+   return value is always an open file.  */
 static FILE *
-m4_tmpfile (void)
+m4_tmpfile (int divnum)
 {
-  static unsigned int count;
-  char *name;
+  const char *name;
   FILE *file;
 
   if (output_temp_dir == NULL)
@@ -201,17 +219,56 @@
 		  "cannot create temporary file for diversion"));
       atexit (cleanup_tmpfile);
     }
-  name = xasprintf ("%s/m4-%d", output_temp_dir->dir_name, count++);
+  name = m4_tmpname (divnum);
   register_temp_file (output_temp_dir, name);
   errno = 0;
   file = fopen_temp (name, O_BINARY ? "wb+" : "w+");
   if (file == NULL)
+    {
+      unregister_temp_file (output_temp_dir, name);
+      M4ERROR ((EXIT_FAILURE, errno,
+		"cannot create temporary file for diversion"));
+    }
+  else if (set_cloexec_flag (fileno (file), true) != 0)
+    M4ERROR ((warning_status, errno,
+	      "Warning: cannot protect diversion across forks"));
+  return file;
+}
+
+/* Reopen a temporary file for diversion DIVNUM for reading and
+   writing in a secure temp directory.  Exits on failure, so the
+   return value is always an open file.  */
+static FILE *
+m4_tmpopen (int divnum)
+{
+  const char *name = m4_tmpname (divnum);
+  FILE *file;
+
+  errno = 0;
+  file = fopen_temp (name, O_BINARY ? "ab+" : "a+");
+  if (file == NULL)
     M4ERROR ((EXIT_FAILURE, errno,
 	      "cannot create temporary file for diversion"));
-  free (name);
+  else if (set_cloexec_flag (fileno (file), true) != 0)
+    M4ERROR ((warning_status, errno,
+	      "Warning: cannot protect diversion across forks"));
   return file;
 }
 
+/* Close, but don't delete, a temporary FILE.  */
+static int
+m4_tmpclose (FILE *file)
+{
+  return close_stream_temp (file);
+}
+
+/* Delete a closed temporary FILE for diversion DIVNUM.  */
+static int
+m4_tmpremove (int divnum)
+{
+  return cleanup_temp_file (output_temp_dir, m4_tmpname (divnum));
+}
+
 /*-----------------------------------------------------------------------.
 | Reorganize in-memory diversion buffers so the current diversion can	 |
 | accomodate LENGTH more characters without further reorganization.  The |
@@ -280,11 +337,7 @@
       total_buffer_size -= selected_diversion->size;
       selected_diversion->size = 0;
       selected_diversion->u.file = NULL;
-      selected_diversion->u.file = m4_tmpfile ();
-
-      if (set_cloexec_flag (fileno (selected_diversion->u.file), true) != 0)
-	M4ERROR ((warning_status, errno,
-		  "Warning: cannot protect diversion across forks"));
+      selected_diversion->u.file = m4_tmpfile (selected_diversion->divnum);
 
       if (selected_diversion->used > 0)
 	{
@@ -298,7 +351,7 @@
       /* Reclaim the buffer space for other diversions.  */
 
       free (selected_buffer);
-      selected_diversion->used = 0;
+      selected_diversion->used = 1;
     }
 
   /* Reload output_file, just in case the flushed diversion was current.  */
@@ -313,8 +366,16 @@
     }
   else
     {
-      /* The buffer may be safely reallocated.  */
+      /* Close any selected file since it is not the current diversion.  */
+      if (selected_diversion)
+	{
+	  FILE *file = selected_diversion->u.file;
+	  selected_diversion->u.file = 0;
+	  if (m4_tmpclose (file) != 0)
+	    M4ERROR ((0, errno, "cannot close temporary file for diversion"));
+	}
 
+      /* The current buffer may be safely reallocated.  */
       output_diversion->u.buffer
 	= xrealloc (output_diversion->u.buffer, (size_t) wanted_size);
 
@@ -501,10 +562,18 @@
 	  if (!gl_oset_remove (diversion_table, output_diversion))
 	    error (EXIT_FAILURE, 0, "INTERNAL ERROR: make_diversion failed");
 	  output_diversion->u.next = free_list;
+	  output_diversion->used = 0;
 	  free_list = output_diversion;
 	}
-      else
+      else if (output_diversion->size)
 	output_diversion->used = output_diversion->size - output_unused;
+      else if (output_diversion->used)
+	{
+	  FILE *file = output_diversion->u.file;
+	  output_diversion->u.file = NULL;
+	  if (m4_tmpclose (file) != 0)
+	    M4ERROR ((0, errno, "cannot close temporary file for diversion"));
+	}
       output_diversion = NULL;
       output_file = NULL;
       output_cursor = NULL;
@@ -556,7 +625,11 @@
       output_unused = output_diversion->size - output_diversion->used;
     }
   else
-    output_file = output_diversion->u.file;
+    {
+      if (!output_diversion->u.file && output_diversion->used)
+	output_diversion->u.file = m4_tmpopen (output_diversion->divnum);
+      output_file = output_diversion->u.file;
+    }
   output_current_line = -1;
 }
 
@@ -605,10 +678,15 @@
     {
       if (diversion->size)
 	output_text (diversion->u.buffer, diversion->used);
-      else if (diversion->u.file)
+      else
 	{
-	  rewind (diversion->u.file);
-	  insert_file (diversion->u.file);
+	  if (!diversion->u.file && diversion->used)
+	    diversion->u.file = m4_tmpopen (diversion->divnum);
+	  if (diversion->u.file)
+	    {
+	      rewind (diversion->u.file);
+	      insert_file (diversion->u.file);
+	    }
 	}
 
       output_current_line = -1;
@@ -621,8 +699,19 @@
       diversion->size = 0;
       diversion->used = 0;
     }
-  else if (diversion->u.file && close_stream_temp (diversion->u.file) != 0)
-    M4ERROR ((0, errno, "cannot clean temporary file for diversion"));
+  else
+    {
+      if (diversion->u.file)
+	{
+	  FILE *file = diversion->u.file;
+	  diversion->u.file = NULL;
+	  diversion->used = 0;
+	  if (m4_tmpclose (file) != 0)
+	    M4ERROR ((0, errno, "cannot clean temporary file for diversion"));
+	}
+      if (m4_tmpremove (diversion->divnum) != 0)
+	M4ERROR ((0, errno, "cannot clean temporary file for diversion"));
+    }
   gl_oset_remove (diversion_table, diversion);
   diversion->u.next = free_list;
   free_list = diversion;