Re: 'csplit' core dumps in 'make check'

[Jim Meyering]

address@hidden wrote:
> I consistently see an error in the 'make check' step of coreutils-5.0.
> I reran it with VERBOSE=yes, and I've attached the resulting output
> (csplit.txt).  I also attached the "config.log" file.  Is there anything
> else you might need to recreate the problem?
...
> + csplit in /^$/ 2
> + 1> out
> ./csplit[29]: 18052 Memory fault(coredump)

What type of system were you running that on?
I didn't see such a problem on any of the systems
I've tested on.

Amazing coincidence.
Just today I finished making enough changes to valgrind
so that it can now run the entire coreutils test suite.
In so doing, it detected a problem with csplit.
That code was reading from a malloc'd buffer that'd just
been freed.  I'll bet that's the test was failing on your system.
Here's the patch:

        Fix read-from-free'd-buffer error detected by valgrind.
        * src/csplit.c (remove_line): Don't return a pointer to data in
        a freed buffer.  Instead, arrange to free the buffer on the
        subsequent call.

Index: src/csplit.c
===================================================================
RCS file: /fetish/cu/src/csplit.c,v
retrieving revision 1.123
retrieving revision 1.124
diff -u -p -u -r1.123 -r1.124
--- src/csplit.c        18 Oct 2003 10:05:47 -0000      1.123
+++ src/csplit.c        16 Nov 2003 12:25:35 -0000      1.124
@@ -526,9 +526,20 @@ get_first_line_in_buffer (void)
 static struct cstring *
 remove_line (void)
 {
+  /* If non-NULL, this is the buffer for which the previous call
+     returned the final line.  So now, presuming that line has been
+     processed, we can free the buffer and reset this pointer.  */
+  static struct buffer_record *prev_buf = NULL;
+
   struct cstring *line;                /* Return value. */
   struct line *l;              /* For convenience. */
 
+  if (prev_buf)
+    {
+      free_buffer (prev_buf);
+      prev_buf = NULL;
+    }
+
   if (head == NULL && !load_buffer ())
     return NULL;
 
@@ -548,10 +559,11 @@ remove_line (void)
       head->curr_line = l->next;
       if (head->curr_line == NULL || head->curr_line->used == 0)
        {
-         /* Go on to the next data block. */
-         struct buffer_record *b = head;
+         /* Go on to the next data block.
+            but first record the current one so we can free it
+            once the line we're returning has been processed.  */
+         prev_buf = head;
          head = head->next;
-         free_buffer (b);
        }
     }