Re: branch-1_4 allocation overflow
Eric Blake <[email protected]>
| Newsgroups | gmane.comp.gnu.m4.patches |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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 <[email protected]> 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