sort: simpler fix for sort -u data-loss bug, and for a FMR bug
authorPaul Eggert <eggert@cs.ucla.edu>
Fri, 17 Aug 2012 20:26:00 +0000 (13:26 -0700)
committerJim Meyering <meyering@redhat.com>
Sat, 18 Aug 2012 05:39:29 +0000 (07:39 +0200)
This also fixes a free-memory-read (FMR) bug: when fillbuf's realloc
of buf->buf frees the buffer into which saved_line.text points,
the processing of that just-read longer line includes comparison
against the saved line in freed memory.
* src/sort.c (overlap): Remove.
(fillbuf): Do not try to copy saved lines, as that is too risky
in the presence of parallelism, reallocated buffers, etc.
(sort): Invalidate any saved line before sorting a new batch.

src/sort.c

index c2d2d49..9dbfee1 100644 (file)
@@ -1705,14 +1705,6 @@ limfield (struct line const *line, struct keyfield const *key)
   return ptr;
 }
 
-/* Return true if LINE and the buffer BUF of length LEN overlap.  */
-static inline bool
-overlap (char const *buf, size_t len, struct line const *line)
-{
-  char const *line_end = line->text + line->length;
-  return !(line_end <= buf || buf + len <= line->text);
-}
-
 /* Fill BUF reading from FP, moving buf->left bytes from the end
    of buf->buf to the beginning first.  If EOF is reached and the
    file wasn't terminated by a newline, supply one.  Set up BUF's line
@@ -1753,33 +1745,6 @@ fillbuf (struct buffer *buf, FILE *fp, char const *file)
              rest of the input file consists entirely of newlines,
              except that the last byte is not a newline.  */
           size_t readsize = (avail - 1) / (line_bytes + 1);
-
-          /* With --unique, when we're about to read into a buffer that
-             overlaps the saved "preceding" line (saved_line), copy the line's
-             .text member to a realloc'd-as-needed temporary buffer and adjust
-             the line's key-defining members if they're set.  */
-          if (unique && overlap (ptr, readsize, &saved_line))
-            {
-              /* Copy saved_line.text into a buffer where it won't be clobbered
-                 and if KEY is non-NULL, adjust saved_line.key* to match.  */
-              static char *safe_text;
-              static size_t safe_text_n_alloc;
-              if (safe_text_n_alloc < saved_line.length)
-                {
-                  safe_text_n_alloc = saved_line.length;
-                  safe_text = x2nrealloc (safe_text, &safe_text_n_alloc, 1);
-                }
-              memcpy (safe_text, saved_line.text, saved_line.length);
-              if (key)
-                {
-                  #define s saved_line
-                  s.keybeg = safe_text + (s.keybeg - s.text);
-                  s.keylim = safe_text + (s.keylim - s.text);
-                  #undef s
-                }
-              saved_line.text = safe_text;
-            }
-
           size_t bytes_read = fread (ptr, 1, readsize, fp);
           char *ptrlim = ptr + bytes_read;
           char *p;
@@ -3928,6 +3893,7 @@ sort (char *const *files, size_t nfiles, char const *output_file,
               break;
             }
 
+          saved_line.text = NULL;
           line = buffer_linelim (&buf);
           if (buf.eof && !nfiles && !ntemps && !buf.left)
             {