change 'sort' to report error with "sort -R -i", etc.

[Paul Eggert]

In looking over the recent sort -R change, I noticed that -R doesn't
work well when it is combined with several other options.  For
example, "sort -R -i" should hash only the nonprinting characters, but
it isn't implemented that way, so in this case -i is ignored (unless a
hash collision occurs, which is unlikely).

The problem occurs with some previously existing options too, e.g.,
"sort -d -n", or "sort -i -n".

I installed the following change so that "sort" will report
incompatibilities like these.  This will give us room in the future to
support combinations like -i -R correctly, as a pure extension.

A possible downside is that (unportable) shell scripts that invoke
incompatible option combinations like "sort -d -n" will now get an
error rather than silently ignoring the "-d".

As it happens, the POSIX spec isn't 100% clear about what to do with
the particular combination "sort -i -n".  I've filed a bug report
about the "sort -i -n" issue with the Open Group; see
<https://www.opengroup.org/sophocles/show_mail.tpl?source=L&listname=austin-review-l&id=1983>.


2005-12-14  Paul Eggert  <address@hidden>

        * NEWS: sort now reports incompatible options.
        * src/sort.c (incompatible_options, check_ordering_compatibility):
        New functions.
        (main): Use them.  Don't bother with a usage message for
        "sort -c a b", for consistency with other error diagnostics.
        * tests/sort/Test.pm (incompat1, incompat2, incompat3, incompat4):
        New tests.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.351
retrieving revision 1.352
diff -p -u -r1.351 -r1.352
--- NEWS        10 Dec 2005 21:10:21 -0000      1.351
+++ NEWS        14 Dec 2005 22:46:17 -0000      1.352
@@ -23,6 +23,9 @@ GNU coreutils NEWS                      
   if your locale settings appear to be messed up.  This change
   attempts to have the default be the best of both worlds.
 
+  sort now reports incompatible options (e.g., -i and -n) rather than
+  silently ignoring one of them.
+
 ** Scheduled for removal
 
   rm's --directory (-d) option is scheduled for removal in 2006.  This
Index: src/sort.c
===================================================================
RCS file: /fetish/cu/src/sort.c,v
retrieving revision 1.333
retrieving revision 1.335
diff -p -u -r1.333 -r1.335
--- src/sort.c  12 Dec 2005 22:09:27 -0000      1.333
+++ src/sort.c  14 Dec 2005 23:59:23 -0000      1.335
@@ -2046,6 +2046,49 @@ badfieldspec (char const *spec, char con
   abort ();
 }
 
+/* Report incompatible options.  */
+
+static void incompatible_options (char const *) ATTRIBUTE_NORETURN;
+static void
+incompatible_options (char const *opts)
+{
+  error (SORT_FAILURE, 0, _("options `-%s' are incompatible"), opts);
+  abort ();
+}
+
+/* Check compatibility of ordering options.  */
+
+static void
+check_ordering_compatibility (void)
+{
+  struct keyfield const *key;
+
+  for (key = keylist; key; key = key->next)
+    if ((1 < (key->random + key->numeric + key->general_numeric + key->month
+             + !!key->ignore))
+       || (key->random && key->translate))
+      {
+       char opts[7];
+       char *p = opts;
+       if (key->ignore == nondictionary)
+         *p++ = 'd';
+       if (key->translate)
+         *p++ = 'f';
+       if (key->general_numeric)
+         *p++ = 'g';
+       if (key->ignore == nonprinting)
+         *p++ = 'i';
+       if (key->month)
+         *p++ = 'M';
+       if (key->numeric)
+         *p++ = 'n';
+       if (key->random)
+         *p++ = 'R';
+       *p = '\0';
+       incompatible_options (opts);
+      }
+}
+
 /* Parse the leading integer in STRING and store the resulting value
    (which must fit into size_t) into *VAL.  Return the address of the
    suffix after the integer.  If MSGID is NULL, return NULL after
@@ -2510,6 +2553,8 @@ main (int argc, char **argv)
       need_random |= gkey.random;
     }
 
+  check_ordering_compatibility ();
+
   reverse = gkey.reverse;
 
   if (need_random)
@@ -2538,11 +2583,11 @@ main (int argc, char **argv)
   if (checkonly)
     {
       if (nfiles > 1)
-       {
-         error (0, 0, _("extra operand %s not allowed with -c"),
-                quote (files[1]));
-         usage (SORT_FAILURE);
-       }
+       error (SORT_FAILURE, 0, _("extra operand %s not allowed with -c"),
+              quote (files[1]));
+
+      if (outfile)
+       incompatible_options ("co");
 
       /* POSIX requires that sort return 1 IFF invoked with -c and the
         input is not properly sorted.  */
Index: tests/sort/Test.pm
===================================================================
RCS file: /fetish/cu/tests/sort/Test.pm,v
retrieving revision 1.30
retrieving revision 1.32
diff -p -u -r1.30 -r1.32
--- tests/sort/Test.pm  29 Apr 2005 23:50:52 -0000      1.30
+++ tests/sort/Test.pm  14 Dec 2005 23:58:20 -0000      1.32
@@ -247,6 +247,12 @@ my @tv = (
 # Specifying two -o options should evoke a failure
 ["o2", '-o x -o y', '', '', 2],
 
+# Specifying incompatible options should evoke a failure.
+["incompat1", '-in', '', '', 2],
+["incompat2", '-fR', '', '', 2],
+["incompat3", '-dfgiMnR', '', '', 2],
+["incompat4", '-c -o /dev/null', '', '', 2],
+
 # -t '\0' is accepted, as of coreutils-5.0.91
 ['nul-tab', "-k2,2 -t '\\0'", "a\0z\01\nb\0y\02\n", "b\0y\02\na\0z\01\n", 0],
 );