[Top][All Lists]

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

bug#5958: Sort-8.4 bug

From: Alan Curry
Subject: bug#5958: Sort-8.4 bug
Date: Tue, 20 Apr 2010 03:37:33 -0500 (GMT+5)

A full investigation has revealed:

This bug was introduced between coreutils 7.1 and 7.2, here:

>commit 224a69b56b716f57e3a018af5a9b9379f32da3fc
>Author: Pádraig Brady <address@hidden>
>Date:   Tue Feb 24 08:37:18 2009 +0000
>    sort: Fix two bugs with determining the end of field
>    * src/sort.c: When no specific number of chars to skip
>    is specified for the end field, always skip the whole field.
>    Also never include leading spaces from next field.
>    * tests/misc/sort: Add 2 new tests for these cases.
>    * NEWS: Mention this bug fix.
>    * THANKS: Add bug reporter.
>    Reported by Davide Canova.

In the diff of that commit, an eword++ was removed from the case 'k' section
of option parsing, where it did not affect traditional options, and added to
the limfield() function, where it takes effect regardless of how fields were

So it fixed a -k option parsing bug and added a traditional option parsing
bug. And on the way, it removed a comment describing the correct
correspondence between the two!

The following patch moves the eword++ back to its old location (under the
case 'k') but keeps the new test for when it should be applied (echar==0,
whether by explicit .0 on the field end specifier or by omission of the field
end specifier). This allows the -k bug that was fixed to stay fixed, while
undoing the damage to the traditional options.

With this patch applied, all the sort tests in make check still pass,
including the tests added in the above commit, which I take as a sign that I
got it right. And the traditional options are back to working again.

I'd suggest the following new test case:

printf "a b c\na c b\n" | sort +0 -1 +2

should output "a c b\na b c\n"

I'd put that in the diff too, but the organization of tests/misc/sort is

--- coreutils-8.4.orig/src/sort.c       2010-04-20 02:45:35.000000000 -0500
+++ coreutils-8.4/src/sort.c    2010-04-20 03:12:57.000000000 -0500
@@ -1460,9 +1460,6 @@
   char *ptr = line->text, *lim = ptr + line->length - 1;
   size_t eword = key->eword, echar = key->echar;
-  if (echar == 0)
-    eword++; /* Skip all of end field.  */
   /* Move PTR past EWORD fields or to one past the last byte on LINE,
      whichever comes first.  If there are more than EWORD fields, leave
      PTR pointing at the beginning of the field having zero-based index,
@@ -3424,6 +3421,8 @@
                   s = parse_field_count (s + 1, &key->echar,
                                          N_("invalid number after `.'"));
+              if (key->echar == 0)
+                key->eword++; /* Skip all of end field.  */
               s = set_ordering (s, key, bl_end);
           if (*s)

OK now let's not say I haven't done any legwork.

Alan Curry

reply via email to

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