m4-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: branch-1_4 allocation overflow


From: Eric Blake
Subject: Re: branch-1_4 allocation overflow
Date: Thu, 26 Oct 2006 23:18:28 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

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

> 
> On head, I wonder if it would be nicer to use a sorted list of active
> diversions, rather than allocating such a sparse array, so that memory is
> only allocated for used diversions.

Here's the two-part patch I am using for head.  First I optimized the data 
representation, and got rid of a number of potential size_t-to-int bugs on 64-
bit platforms.  The second part pulls in a gnulib list structure, to convert 
the sparse array into an efficient AVL tree.


2006-10-26  Eric Blake  <address@hidden>

        Convert diversions to list instead of sparse array, part 1.
        * m4/m4module.h (m4_shipout_text, m4_shipout_string): Use size_t
        for length.
        * m4/output.c (diversion, total_buffer_size, output_unused):
        Track size in size_t.  All users changed.
        (struct diversion): Reduce size now, to compensate for part 2.
        (m4_make_diversion, make_room_for): Avoid malloc overflow.
        (m4_output_exit): Fix typo in assert.
        (m4_output_init, make_room_for, m4_make_diversion)
        (m4_insert_diversion, m4_freeze_diversions): Adjust users of
        struct m4_diversion to disambiguate the new union.
        * tests/builtins.at (divert): Detect this failure.


Index: m4/m4module.h
===================================================================
RCS file: /sources/m4/m4/m4/m4module.h,v
retrieving revision 1.96
diff -u -p -r1.96 m4module.h
--- m4/m4module.h       25 Oct 2006 23:19:19 -0000      1.96
+++ m4/m4module.h       26 Oct 2006 20:49:17 -0000
@@ -413,10 +413,10 @@ extern    void    m4_input_print  (m4 *, m4_obs
 
 extern void    m4_output_init    (m4 *);
 extern void    m4_output_exit    (void);
-extern void    m4_shipout_text   (m4 *, m4_obstack *, const char *, int);
+extern void    m4_shipout_text   (m4 *, m4_obstack *, const char *, size_t);
 extern void    m4_shipout_int    (m4_obstack *, int);
 extern void    m4_shipout_string (m4 *, m4_obstack *, const char *,
-                                  int, bool);
+                                  size_t, bool);
 
 extern void    m4_make_diversion    (m4 *, int);
 extern void    m4_insert_diversion  (m4 *, int);
Index: m4/output.c
===================================================================
RCS file: /sources/m4/m4/m4/output.c,v
retrieving revision 1.32
diff -u -p -r1.32 output.c
--- m4/output.c 7 Oct 2006 05:02:56 -0000       1.32
+++ m4/output.c 26 Oct 2006 20:49:17 -0000
@@ -55,35 +55,40 @@
 
 typedef struct temp_dir m4_temp_dir;
 
-/* In a struct diversion, only one of file or buffer be may non-NULL,
-   depending on the fact output is diverted to a file or in memory
-   buffer.  Further, if buffer is NULL, then pointer is NULL, size and
-   unused are zero.  */
+/* When part of diversion_table, each struct 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).  */
 
-struct diversion
+typedef struct m4_diversion m4_diversion;
+
+struct m4_diversion
   {
-    FILE *file;                        /* diversion file on disk */
-    char *buffer;              /* in-memory diversion buffer */
-    int size;                  /* usable size before reallocation */
-    int used;                  /* used length in characters */
+    union
+      {
+       FILE *file;             /* diversion file on disk */
+       char *buffer;           /* in-memory diversion buffer */
+      } u;
+    size_t size;               /* usable size before reallocation */
+    size_t used;               /* used length in characters */
   };
 
 /* Table of diversions.  */
-static struct diversion *diversion_table;
+static m4_diversion *diversion_table;
 
 /* Number of entries in diversion table.  */
 static int diversions;
 
 /* Total size of all in-memory buffer sizes.  */
-static int total_buffer_size;
+static size_t total_buffer_size;
 
 /* Current output diversion, NULL if output is being currently discarded.  */
-static struct diversion *output_diversion;
+static m4_diversion *output_diversion;
 
 /* Values of some output_diversion fields, cached out for speed.  */
 static FILE *output_file;      /* current value of (file) */
 static char *output_cursor;    /* current value of (buffer + used) */
-static int output_unused;      /* current value of (size - used) */
+static size_t output_unused;   /* current value of (size - used) */
 
 /* Temporary directory holding all spilled diversion files.  */
 static m4_temp_dir *output_temp_dir;
@@ -97,8 +102,7 @@ m4_output_init (m4 *context)
 {
   diversion_table = xmalloc (sizeof *diversion_table);
   diversions = 1;
-  diversion_table[0].file = stdout;
-  diversion_table[0].buffer = NULL;
+  diversion_table[0].u.file = stdout;
   diversion_table[0].size = 0;
   diversion_table[0].used = 0;
 
@@ -113,7 +117,12 @@ m4_output_init (m4 *context)
 void
 m4_output_exit (void)
 {
-  assert (diversions = 1);
+#ifndef NDEBUG
+  int divnum;
+  for (divnum = 1; divnum < diversions; divnum++)
+    assert (!diversion_table[divnum].size
+           && !diversion_table[divnum].u.file);
+#endif
   DELETE (diversion_table);
 }
 
@@ -166,9 +175,14 @@ m4_tmpfile (m4 *context)
    to be flushed to a newly created temporary file.  This flushed buffer
    might well be the current one.  */
 static void
-make_room_for (m4 *context, int length)
+make_room_for (m4 *context, size_t length)
 {
-  int wanted_size;
+  size_t wanted_size;
+  m4_diversion *selected_diversion = NULL;
+
+  assert (!output_file);
+  assert (output_diversion);
+  assert (output_diversion->size || !output_diversion->u.file);
 
   /* Compute needed size for in-memory buffer.  Diversions in-memory
      buffers start at 0 bytes, then 512, then keep doubling until it is
@@ -177,7 +191,8 @@ make_room_for (m4 *context, int length)
   output_diversion->used = output_diversion->size - output_unused;
 
   for (wanted_size = output_diversion->size;
-       wanted_size < output_diversion->used + length;
+       wanted_size <= MAXIMUM_TOTAL_SIZE
+        && wanted_size - output_diversion->used < length;
        wanted_size = wanted_size == 0 ? INITIAL_BUFFER_SIZE : wanted_size * 2)
     ;
 
@@ -186,10 +201,10 @@ make_room_for (m4 *context, int length)
   if (total_buffer_size - output_diversion->size + wanted_size
       > MAXIMUM_TOTAL_SIZE)
     {
-      struct diversion *selected_diversion;
-      int selected_used;
-      struct diversion *diversion;
-      int count;
+      size_t selected_used;
+      char *selected_buffer;
+      m4_diversion *diversion;
+      size_t count;
 
       /* Find out the buffer having most data, in view of flushing it to
         disk.  Fake the current buffer as having already received the
@@ -211,17 +226,16 @@ make_room_for (m4 *context, int length)
       /* Create a temporary file, write the in-memory buffer of the
         diversion to this file, then release the buffer.  */
 
-      selected_diversion->file = m4_tmpfile (context);
-      if (set_cloexec_flag (fileno (selected_diversion->file), true) != 0)
+      selected_buffer = selected_diversion->u.buffer;
+      selected_diversion->u.file = m4_tmpfile (context);
+      if (set_cloexec_flag (fileno (selected_diversion->u.file), true) != 0)
        m4_error (context, 0, errno,
                  _("cannot protect diversion across forks"));
 
       if (selected_diversion->used > 0)
        {
-         count = fwrite (selected_diversion->buffer,
-                         (size_t) selected_diversion->used,
-                         1,
-                         selected_diversion->file);
+         count = fwrite (selected_buffer, selected_diversion->used, 1,
+                         selected_diversion->u.file);
          if (count != 1)
            m4_error (context, EXIT_FAILURE, errno,
                      _("cannot flush diversion to temporary file"));
@@ -229,37 +243,36 @@ make_room_for (m4 *context, int length)
 
       /* Reclaim the buffer space for other diversions.  */
 
-      free (selected_diversion->buffer);
+      free (selected_buffer);
       total_buffer_size -= selected_diversion->size;
 
-      selected_diversion->buffer = NULL;
       selected_diversion->size = 0;
       selected_diversion->used = 0;
     }
 
   /* Reload output_file, just in case the flushed diversion was current.  */
 
-  output_file = output_diversion->file;
-  if (output_file)
+  if (output_diversion == selected_diversion)
     {
 
       /* The flushed diversion was current indeed.  */
 
+      output_file = output_diversion->u.file;
       output_cursor = NULL;
       output_unused = 0;
     }
   else
     {
-
       /* The buffer may be safely reallocated.  */
 
-      output_diversion->buffer
-       = xrealloc (output_diversion->buffer, (size_t) wanted_size);
+      output_diversion->u.buffer
+       = xrealloc (output_diversion->u.buffer,
+                   wanted_size > length ? wanted_size : length);
 
       total_buffer_size += wanted_size - output_diversion->size;
       output_diversion->size = wanted_size;
 
-      output_cursor = output_diversion->buffer + output_diversion->used;
+      output_cursor = output_diversion->u.buffer + output_diversion->used;
       output_unused = wanted_size - output_diversion->used;
     }
 }
@@ -292,9 +305,9 @@ output_character_helper (m4 *context, in
 /* Output one TEXT having LENGTH characters, when it is known that it goes
    to a diversion file or an in-memory diversion buffer.  */
 static void
-output_text (m4 *context, const char *text, int length)
+output_text (m4 *context, const char *text, size_t length)
 {
-  int count;
+  size_t count;
 
   if (!output_file && length > output_unused)
     make_room_for (context, length);
@@ -307,7 +320,7 @@ output_text (m4 *context, const char *te
     }
   else
     {
-      memcpy (output_cursor, text, (size_t) length);
+      memcpy (output_cursor, text, length);
       output_cursor += length;
       output_unused -= length;
     }
@@ -324,7 +337,7 @@ output_text (m4 *context, const char *te
    output lines, or when several input lines does not generate any output.  */
 void
 m4_shipout_text (m4 *context, m4_obstack *obs,
-                const char *text, int length)
+                const char *text, size_t length)
 {
   static bool start_of_output_line = true;
   char line[20];
@@ -376,9 +389,9 @@ m4_shipout_text (m4 *context, m4_obstack
            m4_set_output_line (context, m4_get_output_line (context) + 1);
 
 #ifdef DEBUG_OUTPUT
-           fprintf (stderr, "DEBUG: cur %d, cur out %d\n",
-                     m4_get_current_line (context),
-                     m4_get_output_line (context));
+           fprintf (stderr, "DEBUG: cur %lu, cur out %lu\n",
+                    (unsigned long int) m4_get_current_line (context),
+                    (unsigned long int) m4_get_output_line (context));
 #endif
 
            /* Output a `#line NUM' synchronization directive if needed.
@@ -387,7 +400,8 @@ m4_shipout_text (m4 *context, m4_obstack
 
            if (m4_get_output_line (context) != m4_get_current_line (context))
              {
-               sprintf (line, "#line %d", m4_get_current_line (context));
+               sprintf (line, "#line %lu",
+                        (unsigned long int) m4_get_current_line (context));
                for (cursor = line; *cursor; cursor++)
                  OUTPUT_CHARACTER (*cursor);
                if (m4_get_output_line (context) < 1
@@ -412,8 +426,9 @@ m4_shipout_text (m4 *context, m4_obstack
       }
 }
 
-/* Format an int VAL, and stuff it into an obstack OBS.  Used for macros
-   expanding to numbers.  */
+/* Format an int VAL, and stuff it into an obstack OBS.  Used for
+   macros expanding to numbers.  FIXME - support wider types, and
+   unsigned types.  */
 void
 m4_shipout_int (m4_obstack *obs, int val)
 {
@@ -424,7 +439,7 @@ m4_shipout_int (m4_obstack *obs, int val
 }
 
 void
-m4_shipout_string (m4 *context, m4_obstack *obs, const char *s, int len,
+m4_shipout_string (m4 *context, m4_obstack *obs, const char *s, size_t len,
                   bool quoted)
 {
   if (s == NULL)
@@ -455,11 +470,11 @@ m4_shipout_string (m4 *context, m4_obsta
 void
 m4_make_diversion (m4 *context, int divnum)
 {
-  struct diversion *diversion;
+  m4_diversion *diversion;
 
   if (output_diversion)
     {
-      output_diversion->file = output_file;
+      assert (!output_file || output_diversion->u.file == output_file);
       output_diversion->used = output_diversion->size - output_unused;
       output_diversion = NULL;
       output_file = NULL;
@@ -474,14 +489,14 @@ m4_make_diversion (m4 *context, int divn
 
   if (divnum >= diversions)
     {
-      diversion_table = (struct diversion *)
-       xrealloc (diversion_table, (divnum + 1) * sizeof (struct diversion));
+      diversion_table = (m4_diversion *) xnrealloc (diversion_table,
+                                                   divnum + 1,
+                                                   sizeof (*diversion_table));
       for (diversion = diversion_table + diversions;
           diversion <= diversion_table + divnum;
           diversion++)
        {
-         diversion->file = NULL;
-         diversion->buffer = NULL;
+         diversion->u.file = NULL;
          diversion->size = 0;
          diversion->used = 0;
        }
@@ -489,9 +504,14 @@ m4_make_diversion (m4 *context, int divn
     }
 
   output_diversion = diversion_table + divnum;
-  output_file = output_diversion->file;
-  output_cursor = output_diversion->buffer + output_diversion->used;
-  output_unused = output_diversion->size - output_diversion->used;
+  if (output_diversion->size)
+    {
+      output_cursor = output_diversion->u.buffer + output_diversion->used;
+      output_unused = output_diversion->size - output_diversion->used;
+    }
+  else
+    output_file = output_diversion->u.file;
+
   m4_set_output_line (context, -1);
 }
 
@@ -527,7 +547,7 @@ m4_insert_file (m4 *context, FILE *file)
 void
 m4_insert_diversion (m4 *context, int divnum)
 {
-  struct diversion *diversion;
+  m4_diversion *diversion;
 
   /* Do not care about unexisting diversions.  */
 
@@ -544,31 +564,28 @@ m4_insert_diversion (m4 *context, int di
 
   if (output_diversion)
     {
-      if (diversion->file)
+      if (diversion->size)
+       output_text (context, diversion->u.buffer, diversion->used);
+      else if (diversion->u.file)
        {
-         rewind (diversion->file);
-         m4_insert_file (context, diversion->file);
+         rewind (diversion->u.file);
+         m4_insert_file (context, diversion->u.file);
        }
-      else if (diversion->buffer)
-       output_text (context, diversion->buffer, diversion->used);
 
       m4_set_output_line (context, -1);
     }
 
   /* Return all space used by the diversion.  */
 
-  if (diversion->file)
-    {
-      close_stream_temp (diversion->file);
-      diversion->file = NULL;
-    }
-  else if (diversion->buffer)
+  if (diversion->size)
     {
-      free (diversion->buffer);
-      diversion->buffer = NULL;
+      free (diversion->u.buffer);
       diversion->size = 0;
       diversion->used = 0;
     }
+  else if (diversion->u.file)
+    close_stream_temp (diversion->u.file);
+  diversion->u.file = NULL;
 }
 
 /* Get back all diversions.  This is done just before exiting from main (),
@@ -589,7 +606,7 @@ m4_freeze_diversions (m4 *context, FILE 
   int saved_number;
   int last_inserted;
   int divnum;
-  struct diversion *diversion;
+  m4_diversion *diversion;
   struct stat file_stat;
 
   saved_number = m4_get_current_diversion (context);
@@ -600,14 +617,22 @@ m4_freeze_diversions (m4 *context, FILE 
   for (divnum = 1; divnum < diversions; divnum++)
     {
       diversion = diversion_table + divnum;
-      if (diversion->file || diversion->buffer)
+      if (diversion->size || diversion->u.file)
        {
-         if (diversion->file)
+         if (diversion->size)
            {
-             fflush (diversion->file);
-             if (fstat (fileno (diversion->file), &file_stat) < 0)
+             assert (diversion->used == (int) diversion->used);
+             fprintf (file, "D%d,%d\n", divnum, (int) diversion->used);
+           }
+         else
+           {
+             fflush (diversion->u.file);
+             if (fstat (fileno (diversion->u.file), &file_stat) < 0)
                m4_error (context, EXIT_FAILURE, errno,
                          _("cannot stat diversion"));
+             /* FIXME - support 64-bit off_t with 32-bit long, and
+                fix frozen file format to support 64-bit
+                integers.  */
              if (file_stat.st_size < 0
                  || file_stat.st_size != (unsigned long) file_stat.st_size)
                m4_error (context, EXIT_FAILURE, errno,
@@ -615,8 +640,6 @@ m4_freeze_diversions (m4 *context, FILE 
              fprintf (file, "D%d,%lu", divnum,
                       (unsigned long) file_stat.st_size);
            }
-         else
-           fprintf (file, "D%d,%d\n", divnum, diversion->used);
 
          m4_insert_diversion (context, divnum);
          putc ('\n', file);
Index: tests/builtins.at
===================================================================
RCS file: /sources/m4/m4/tests/builtins.at,v
retrieving revision 1.26
diff -u -p -r1.26 builtins.at
--- tests/builtins.at   21 Oct 2006 22:15:52 -0000      1.26
+++ tests/builtins.at   26 Oct 2006 20:49:17 -0000
@@ -161,6 +161,22 @@ AT_CHECK_M4([divert.m4], 0,
 Text diverted a second time.
 ]])
 
+dnl Test for allocation overflow.  On 32-bit platforms, this used to
+dnl coredump due to wraparound allocating a smaller array, then referencing
+dnl unallocated memory; it should now cleanly fail outright.  On 64-bit
+dnl platforms, failure is determined by the amount of memory.  If the
+dnl allocation succeeds (hopefully, the testers don't mind the memory
+dnl thrashing), then fake the same exit symptoms as on 32-bit so that the
+dnl test will still pass.
+AT_DATA([[in]], [[divert(eval(`1<<28'))
+divert(`2')
+errprint(__program__`: memory exhausted
+')m4exit(`1')
+]])
+AT_CHECK_M4([in], [1], [],
+[[m4: memory exhausted
+]])
+
 AT_CLEANUP
 
 


2006-10-26  Eric Blake  <address@hidden>

        Convert diversions to list instead of sparse array, part 2.
        * ltdl/m4/gnulib-cache.m4: Augment with 'gnulib-tool --import
        avltree-list'.
        * m4/output.c (m4_diversion): Add next pointer and divnum members.
        (diversion_table): Convert to a list instead of a sparse array.
        (free_list): Maintain free list of reclaimed diversions.
        (equal_diversion_CB, cmp_diversion_CB): New functions.
        (m4_output_init): Set up list.
        (m4_output_exit): Tear down list.
        (make_room_for, m4_undivert_all, m4_freeze_diversions): Change
        list iteration.
        (m4_make_diversion): Change creation of new diversions.
        (m4_insert_diversion_helper): New function, to avoid list
        searches.
        * tests/builtins.at (divert): The test now passes.
        * NEWS: Document this improvement.

Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.29
diff -u -r1.29 NEWS
--- NEWS        21 Oct 2006 22:15:52 -0000      1.29
+++ NEWS        26 Oct 2006 23:12:22 -0000
@@ -127,6 +127,11 @@
   override all earlier options.  For example, `m4 -otrace --help' now no
   longer accidentally creates an empty file `trace'.
 
+* The builtin `divert' now uses memory proportional to the number of
+  diversions created, rather than to the maximum diversion number
+  encountered, allowing larger diversion numbers without exhausting system
+  memory.
+
 * FIXME: `m4wrap' semantics need an update to FIFO.
 
 * FIXME: include the (long) list of changes in 1.4.x that were not already
Index: ltdl/m4/gnulib-cache.m4
===================================================================
RCS file: /sources/m4/m4/ltdl/m4/gnulib-cache.m4,v
retrieving revision 1.15
diff -u -r1.15 gnulib-cache.m4
--- ltdl/m4/gnulib-cache.m4     16 Oct 2006 22:12:07 -0000      1.15
+++ ltdl/m4/gnulib-cache.m4     26 Oct 2006 23:12:22 -0000
@@ -15,11 +15,11 @@
 
 
 # Specification in the form of a command-line invocation:
-#   gnulib-tool --import --dir=. --lib=libgnu --source-base=gnu --m4-
base=ltdl/m4 --doc-base=doc --aux-dir=ltdl/config --libtool --macro-prefix=M4 
assert binary-io clean-temp cloexec close-stream closeout configmake dirname 
error exit fdl filenamecat fopen-safer free gendocs gettext gnupload mkstemp 
obstack progname regex regexprops-generic stdbool stdlib-safer strnlen strtol 
unlocked-io verror xalloc xalloc-die xstrndup xstrtol xvasprintf
+#   gnulib-tool --import --dir=. --lib=libgnu --source-base=gnu --m4-
base=ltdl/m4 --doc-base=doc --aux-dir=ltdl/config --libtool --macro-prefix=M4 
assert avltree-list binary-io clean-temp cloexec close-stream closeout 
configmake dirname error exit fdl filenamecat fopen-safer free gendocs gettext 
gnupload mkstemp obstack progname regex regexprops-generic stdbool stdlib-safer 
strnlen strtol unlocked-io verror xalloc xalloc-die xstrndup xstrtol xvasprintf
 
 # Specification in the form of a few gnulib-tool.m4 macro invocations:
 gl_LOCAL_DIR([])
-gl_MODULES([assert binary-io clean-temp cloexec close-stream closeout 
configmake dirname error exit fdl filenamecat fopen-safer free gendocs gettext 
gnupload mkstemp obstack progname regex regexprops-generic stdbool stdlib-safer 
strnlen strtol unlocked-io verror xalloc xalloc-die xstrndup xstrtol 
xvasprintf])
+gl_MODULES([assert avltree-list binary-io clean-temp cloexec close-stream 
closeout configmake dirname error exit fdl filenamecat fopen-safer free gendocs 
gettext gnupload mkstemp obstack progname regex regexprops-generic stdbool 
stdlib-safer strnlen strtol unlocked-io verror xalloc xalloc-die xstrndup 
xstrtol xvasprintf])
 gl_AVOID([])
 gl_SOURCE_BASE([gnu])
 gl_M4_BASE([ltdl/m4])
Index: m4/output.c
===================================================================
RCS file: /sources/m4/m4/m4/output.c,v
retrieving revision 1.33
diff -u -r1.33 output.c
--- m4/output.c 26 Oct 2006 23:11:41 -0000      1.33
+++ m4/output.c 26 Oct 2006 23:12:22 -0000
@@ -30,6 +30,7 @@
 
 #include "binary-io.h"
 #include "clean-temp.h"
+#include "gl_avltree_list.h"
 #include "exitfail.h"
 #include "xvasprintf.h"
 
@@ -58,7 +59,8 @@
 /* When part of diversion_table, each struct 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).  */
+   diversion (zero size, u is NULL).  When not part of
+   diversion_table, u.next is a pointer to the free_list chain.  */
 
 typedef struct m4_diversion m4_diversion;
 
@@ -68,17 +70,22 @@
       {
        FILE *file;             /* diversion file on disk */
        char *buffer;           /* in-memory diversion buffer */
+       m4_diversion *next;     /* free-list pointer */
       } u;
+    int divnum;                        /* which diversion this represents */
     size_t size;               /* usable size before reallocation */
     size_t used;               /* used length in characters */
   };
 
-/* Table of diversions.  */
-static m4_diversion *diversion_table;
+/* Sorted list of current diversions.  */
+static gl_list_t diversion_table;
 
 /* Number of entries in diversion table.  */
 static int diversions;
 
+/* Linked list of reclaimed diversion storage.  */
+static m4_diversion *free_list;
+
 /* Total size of all in-memory buffer sizes.  */
 static size_t total_buffer_size;
 
@@ -97,33 +104,58 @@
 
 /* --- OUTPUT INITIALIZATION --- */
 
+/* Callback for comparing list elements ELT1 and ELT2 for equality in
+   diversion_table.  */
+static bool
+equal_diversion_CB (const void *elt1, const void *elt2)
+{
+  const m4_diversion *d1 = (const m4_diversion *) elt1;
+  const m4_diversion *d2 = (const m4_diversion *) elt2;
+  return d1->divnum == d2->divnum;
+}
+
+/* Callback for comparing list elements ELT1 and ELT2 for order in
+   diversion_table.  */
+static int
+cmp_diversion_CB (const void *elt1, const void *elt2)
+{
+  const m4_diversion *d1 = (const m4_diversion *) elt1;
+  const m4_diversion *d2 = (const m4_diversion *) elt2;
+  /* No need to worry about overflow, since we don't create diversions
+     with negative divnum.  */
+  return d1->divnum - d2->divnum;
+}
+
 void
 m4_output_init (m4 *context)
 {
-  diversion_table = xmalloc (sizeof *diversion_table);
-  diversions = 1;
-  diversion_table[0].u.file = stdout;
-  diversion_table[0].size = 0;
-  diversion_table[0].used = 0;
+  m4_diversion *diversion = xmalloc (sizeof *diversion);
+  diversion->u.file = stdout;
+  diversion->divnum = 0;
+  diversion->size = 0;
+  diversion->used = 0;
+  diversion_table = gl_list_create (GL_AVLTREE_LIST, equal_diversion_CB, NULL,
+                                   false, 1, (const void **) &diversion);
 
-  total_buffer_size = 0;
+  diversions = 1;
   m4_set_current_diversion (context, 0);
-  output_diversion = diversion_table;
+  output_diversion = diversion;
   output_file = stdout;
-  output_cursor = NULL;
-  output_unused = 0;
 }
 
 void
 m4_output_exit (void)
 {
-#ifndef NDEBUG
-  int divnum;
-  for (divnum = 1; divnum < diversions; divnum++)
-    assert (!diversion_table[divnum].size
-           && !diversion_table[divnum].u.file);
-#endif
-  DELETE (diversion_table);
+  m4_diversion *diversion = free_list;
+  assert (gl_list_size (diversion_table) == 1);
+  free ((void *) gl_list_get_at (diversion_table, 0));
+  gl_list_free (diversion_table);
+  while (diversion)
+    {
+      m4_diversion *stale = diversion;
+      diversion = diversion->u.next;
+      free (stale);
+    }
 }
 
 /* Clean up any temporary directory.  Designed for use as an atexit
@@ -205,6 +237,7 @@
       char *selected_buffer;
       m4_diversion *diversion;
       size_t count;
+      gl_list_iterator_t iter;
 
       /* Find out the buffer having most data, in view of flushing it to
         disk.  Fake the current buffer as having already received the
@@ -214,14 +247,15 @@
       selected_diversion = output_diversion;
       selected_used = output_diversion->used + length;
 
-      for (diversion = diversion_table + 1;
-          diversion < diversion_table + diversions;
-          diversion++)
+      iter = gl_list_iterator_from_to (diversion_table, 1,
+                                      gl_list_size (diversion_table));
+      while (gl_list_iterator_next (&iter, (const void **) &diversion, NULL))
        if (diversion->used > selected_used)
          {
            selected_diversion = diversion;
            selected_used = diversion->used;
          }
+      gl_list_iterator_free (&iter);
 
       /* Create a temporary file, write the in-memory buffer of the
         diversion to this file, then release the buffer.  */
@@ -265,9 +299,9 @@
     {
       /* The buffer may be safely reallocated.  */
 
-      output_diversion->u.buffer
-       = xrealloc (output_diversion->u.buffer,
-                   wanted_size > length ? wanted_size : length);
+      assert (wanted_size > length);
+      output_diversion->u.buffer = xrealloc (output_diversion->u.buffer,
+                                            wanted_size);
 
       total_buffer_size += wanted_size - output_diversion->size;
       output_diversion->size = wanted_size;
@@ -471,10 +505,15 @@
 m4_make_diversion (m4 *context, int divnum)
 {
   m4_diversion *diversion;
+  gl_list_node_t node = NULL;
+
+  if (m4_get_current_diversion (context) == divnum)
+    return;
 
   if (output_diversion)
     {
       assert (!output_file || output_diversion->u.file == output_file);
+      assert (output_diversion->divnum != divnum);
       output_diversion->used = output_diversion->size - output_unused;
       output_diversion = NULL;
       output_file = NULL;
@@ -488,22 +527,36 @@
     return;
 
   if (divnum >= diversions)
+    diversions = divnum + 1;
+  else
+    {
+      m4_diversion temp;
+      temp.divnum = divnum;
+      node = gl_sortedlist_search (diversion_table, cmp_diversion_CB, &temp);
+    }
+  if (node != NULL)
+    diversion = (m4_diversion *) gl_list_node_value (diversion_table, node);
+  else
     {
-      diversion_table = (m4_diversion *) xnrealloc (diversion_table,
-                                                   divnum + 1,
-                                                   sizeof (*diversion_table));
-      for (diversion = diversion_table + diversions;
-          diversion <= diversion_table + divnum;
-          diversion++)
+      /* First time visiting this diversion.  */
+      if (free_list)
+       {
+         diversion = free_list;
+         free_list = diversion->u.next;
+         assert (!diversion->size && !diversion->used);
+       }
+      else
        {
-         diversion->u.file = NULL;
+         diversion = (m4_diversion *) xmalloc (sizeof *diversion);
          diversion->size = 0;
          diversion->used = 0;
        }
-      diversions = divnum + 1;
+      diversion->u.file = NULL;
+      diversion->divnum = divnum;
+      gl_sortedlist_add (diversion_table, cmp_diversion_CB, diversion);
     }
 
-  output_diversion = diversion_table + divnum;
+  output_diversion = diversion;
   if (output_diversion->size)
     {
       output_cursor = output_diversion->u.buffer + output_diversion->used;
@@ -541,27 +594,17 @@
     }
 }
 
-/* Insert diversion number DIVNUM into the current output file.  The
-   diversion is NOT placed on the expansion obstack, because it must not
-   be rescanned.  When the file is closed, it is deleted by the system.  */
-void
-m4_insert_diversion (m4 *context, int divnum)
+/* Insert DIVERSION living at NODE into the current output file.  The
+   diversion is NOT placed on the expansion obstack, because it must
+   not be rescanned.  When the file is closed, it is deleted by the
+   system.  */
+static void
+m4_insert_diversion_helper (m4 *context, m4_diversion *diversion,
+                           gl_list_node_t node)
 {
-  m4_diversion *diversion;
-
-  /* Do not care about unexisting diversions.  */
-
-  if (divnum <= 0 || divnum >= diversions)
-    return;
-
-  /* Also avoid undiverting into self.  */
-
-  diversion = diversion_table + divnum;
-  if (diversion == output_diversion)
-    return;
-
+  assert (diversion->divnum > 0
+         && diversion->divnum != m4_get_current_diversion (context));
   /* Effectively undivert only if an output stream is active.  */
-
   if (output_diversion)
     {
       if (diversion->size)
@@ -576,7 +619,6 @@
     }
 
   /* Return all space used by the diversion.  */
-
   if (diversion->size)
     {
       free (diversion->u.buffer);
@@ -585,7 +627,32 @@
     }
   else if (diversion->u.file)
     close_stream_temp (diversion->u.file);
-  diversion->u.file = NULL;
+  gl_list_remove_node (diversion_table, node);
+  diversion->u.next = free_list;
+  free_list = diversion;
+}
+
+/* Insert diversion number DIVNUM into the current output file.  The
+   diversion is NOT placed on the expansion obstack, because it must not
+   be rescanned.  When the file is closed, it is deleted by the system.  */
+void
+m4_insert_diversion (m4 *context, int divnum)
+{
+  m4_diversion *diversion;
+  m4_diversion temp;
+  gl_list_node_t node;
+
+  /* Do not care about nonexistent diversions, and undiverting stdout
+     or self is a no-op.  */
+  if (divnum <= 0 || divnum >= diversions
+      || m4_get_current_diversion (context) == divnum)
+    return;
+  temp.divnum = divnum;
+  node = gl_sortedlist_search (diversion_table, cmp_diversion_CB, &temp);
+  if (node == NULL)
+    return;
+  diversion = (m4_diversion *) gl_list_node_value (diversion_table, node);
+  m4_insert_diversion_helper (context, diversion, node);
 }
 
 /* Get back all diversions.  This is done just before exiting from main (),
@@ -593,10 +660,17 @@
 void
 m4_undivert_all (m4 *context)
 {
-  int divnum;
-
-  for (divnum = 1; divnum < diversions; divnum++)
-    m4_insert_diversion (context, divnum);
+  m4_diversion *diversion;
+  gl_list_iterator_t iter;
+  gl_list_node_t node;
+  int divnum = m4_get_current_diversion (context);
+
+  iter = gl_list_iterator_from_to (diversion_table, 1,
+                                  gl_list_size (diversion_table));
+  while (gl_list_iterator_next (&iter, (const void **) &diversion, &node))
+    if (diversion->divnum != divnum)
+      m4_insert_diversion_helper (context, diversion, node);
+  gl_list_iterator_free (&iter);
 }
 
 /* Produce all diversion information in frozen format on FILE.  */
@@ -608,15 +682,18 @@
   int divnum;
   m4_diversion *diversion;
   struct stat file_stat;
+  gl_list_iterator_t iter;
+  gl_list_node_t node;
 
   saved_number = m4_get_current_diversion (context);
   last_inserted = 0;
   m4_make_diversion (context, 0);
   output_file = file;          /* kludge in the frozen file */
 
-  for (divnum = 1; divnum < diversions; divnum++)
+  iter = gl_list_iterator_from_to (diversion_table, 1,
+                                  gl_list_size (diversion_table));
+  while (gl_list_iterator_next (&iter, (const void **) &diversion, &node))
     {
-      diversion = diversion_table + divnum;
       if (diversion->size || diversion->u.file)
        {
          if (diversion->size)
@@ -641,12 +718,13 @@
                       (unsigned long) file_stat.st_size);
            }
 
-         m4_insert_diversion (context, divnum);
+         m4_insert_diversion_helper (context, diversion, node);
          putc ('\n', file);
 
          last_inserted = divnum;
        }
     }
+  gl_list_iterator_free (&iter);
 
   /* Save the active diversion number, if not already.  */
 
Index: tests/builtins.at
===================================================================
RCS file: /sources/m4/m4/tests/builtins.at,v
retrieving revision 1.27
diff -u -r1.27 builtins.at
--- tests/builtins.at   26 Oct 2006 23:11:41 -0000      1.27
+++ tests/builtins.at   26 Oct 2006 23:12:22 -0000
@@ -161,20 +161,13 @@
 Text diverted a second time.
 ]])
 
-dnl Test for allocation overflow.  On 32-bit platforms, this used to
-dnl coredump due to wraparound allocating a smaller array, then referencing
-dnl unallocated memory; it should now cleanly fail outright.  On 64-bit
-dnl platforms, failure is determined by the amount of memory.  If the
-dnl allocation succeeds (hopefully, the testers don't mind the memory
-dnl thrashing), then fake the same exit symptoms as on 32-bit so that the
-dnl test will still pass.
-AT_DATA([[in]], [[divert(eval(`1<<28'))
-divert(`2')
-errprint(__program__`: memory exhausted
-')m4exit(`1')
+dnl Handle large diversions.  In 1.4.7, this caused core dumps; in 1.4.8,
+dnl it exhausted memory on 32-bit platforms.
+AT_DATA([[in]], [[divert(eval(`1<<28'))world
+divert(`2')hello
 ]])
-AT_CHECK_M4([in], [1], [],
-[[m4: memory exhausted
+AT_CHECK_M4([in], [0], [[hello
+world
 ]])
 
 AT_CLEANUP






reply via email to

[Prev in Thread] Current Thread [Next in Thread]