bug#5958: Sort-8.4 bug

[Alan Curry]

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
specified.

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
baffling.

--- 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