cksum: validate options more consistently

We disallow `cksum --tag --check` which is fine,
but the error should be consistent with md5sum,
and less confusing, as it currently mentions
"--binary" and "--text" which weren't specified.

We disallow `cksum --tag --text` which is fine,
but we should also disallow `cksum --text --tag`.

We should honor an explicit --binary (output *)
with this combination of options:
cksum --binary --tag --untagged -a md5 /dev/null

Note this also makes both of `cksum -a md5` and
`cksum --tag -a md5` consistently use binary mode
when reading from a tty on systems like MinGW
where O_BINARY is set.

* src/cksum.c (main): Adjust --text,--binary
and --tag,--untagged option processing.
* tests/cksum/cksum-a.sh: Add test cases.
* tests/cksum/cksum-c.sh: Likewise.
* NEWS: Mention the improvement.
Fixes https://github.com/coreutils/coreutils/issues/163

diff --git a/NEWS b/NEWS
index 240edc562db709d021beac672cc0b855413bb735..822146f9858bf334a7391911bce8d4722f1c03ab 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -70,6 +70,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   is reduced in size by 3.2% through the more efficient reuse of the cksum
   utility by the md5sum and sha*sum utilities.
 
+  'cksum' now validates its options more consistently.
+  E.g., `cksum --text --tag` now fails like `cksum --tag --text` already did.
+
   csplit, ls, and sort, now handle a more complete set of terminating signals.
 
   'du' now processes directories with 10,000 or more entries up to 9 times

diff --git a/src/cksum.c b/src/cksum.c
index 34882267dcca17a8191d14f153a8f0ac593b44cf..2c5cdfbe1c3e692942cb78b84104a47fbefca783 100644 (file)
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -1623,14 +1623,11 @@ main (int argc, char **argv)
         raw_digest = true;
         break;
       case UNTAG_OPTION:
-        if (prefix_tag == 1)
-          binary = -1;
         prefix_tag = 0;
         break;
 # endif
       case TAG_OPTION:
         prefix_tag = 1;
-        binary = 1;
         break;
       case 'z':
         digest_delim = '\0';
@@ -1735,38 +1732,20 @@ main (int argc, char **argv)
    }
 #endif
 
-  if (prefix_tag == -1)
-    prefix_tag = HASH_ALGO_CKSUM;
-
-  if (prefix_tag && !binary)
-   {
-     /* This could be supported in a backwards compatible way
-        by prefixing the output line with a space in text mode.
-        However that's invasive enough that it was agreed to
-        not support this mode with --tag, as --text use cases
-        are adequately supported by the default output format.  */
-#if !HASH_ALGO_CKSUM
-     error (0, 0, _("--tag does not support --text mode"));
-#else
-     error (0, 0, _("--text mode is only supported with --untagged"));
-#endif
-     usage (EXIT_FAILURE);
-   }
-
   if (digest_delim != '\n' && do_check)
     {
       error (0, 0, _("the --zero option is not supported when "
                      "verifying checksums"));
       usage (EXIT_FAILURE);
     }
-#if !HASH_ALGO_CKSUM
-  if (prefix_tag && do_check)
+  if (1 <= prefix_tag && do_check)
     {
+      /* Note we allow --untagged with --check to more
+         seamlessly support --untagged in an emulation wrapper.  */
       error (0, 0, _("the --tag option is meaningless when "
                      "verifying checksums"));
       usage (EXIT_FAILURE);
     }
-#endif
 
   if (0 <= binary && do_check)
     {
@@ -1811,8 +1790,28 @@ main (int argc, char **argv)
      usage (EXIT_FAILURE);
    }
 
+  if (prefix_tag == -1)
+    prefix_tag = HASH_ALGO_CKSUM;
+
+  if (prefix_tag && !binary)
+   {
+     /* This could be supported in a backwards compatible way
+        by prefixing the output line with a space in text mode.
+        However that's invasive enough that it was agreed to
+        not support this mode with --tag, as --text use cases
+        are adequately supported by the default output format.  */
+#if !HASH_ALGO_CKSUM
+     error (0, 0, _("--tag does not support --text mode"));
+#else
+     error (0, 0, _("--text mode is only supported with --untagged"));
+#endif
+     usage (EXIT_FAILURE);
+   }
+
   if (!O_BINARY && binary < 0)
     binary = 0;
+  else if (prefix_tag)
+    binary = 1;
 
   char **operand_lim = argv + argc;
   if (optind == argc)

diff --git a/tests/cksum/cksum-a.sh b/tests/cksum/cksum-a.sh
index e1961efce146aee02998d69d763575a8cfc4d155..ed80a65d3a1f5be504222d622c4bd9e0abd61e5b 100755 (executable)
--- a/tests/cksum/cksum-a.sh
+++ b/tests/cksum/cksum-a.sh
@@ -68,14 +68,15 @@ returns_ 1 cksum -a bsd --check </dev/null || fail=1
 # Ensure abbreviations not supported for algorithm selection
 returns_ 1 cksum -a sha22 </dev/null || fail=1
 
+# Ensure --text and --tag combo is disallowed
+returns_ 1 cksum --text --tag -a md5 </dev/null || fail=1
+returns_ 1 cksum --tag --text -a md5 </dev/null || fail=1
+
 # Ensure --tag -> --untagged transition resets binary indicator
 cksum --tag --untagged -a md5 /dev/null >out-1 || fail=1
-# --binary ignored in this edge case
-cksum --binary --tag --untagged -a md5 /dev/null >out-2 || fail=1
 # base case for comparison
 cksum --untagged -a md5 /dev/null >out || fail=1
 compare out out-1 || fail=1
-compare out out-2 || fail=1
 
 # check that the '*' is present
 cksum --tag --untagged --binary -a md5 /dev/null >out || fail=1
@@ -83,5 +84,7 @@ grep " *" out >/dev/null || { fail=1; cat out; }
 # Verify that the order does not reset binary indicator
 cksum --binary --untagged -a md5 /dev/null >out-1 || fail=1
 compare out out-1 || fail=1
+cksum --binary --tag --untagged -a md5 /dev/null >out-2 || fail=1
+compare out out-2 || fail=1
 
 Exit $fail

diff --git a/tests/cksum/cksum-c.sh b/tests/cksum/cksum-c.sh
index 2aca38043a060945cb309a1f2c39384e79aaf6da..a355bdd63de2fd903a0f6c24d351686623830fbe 100755 (executable)
--- a/tests/cksum/cksum-c.sh
+++ b/tests/cksum/cksum-c.sh
@@ -192,4 +192,13 @@ touch empty-1 empty-2 || framework_failure_
 cksum --check empty-1 empty-2 > out 2>&1 && fail=1
 grep 'empty-1: no properly formatted checksum lines found' out || fail=1
 grep 'empty-2: no properly formatted checksum lines found' out || fail=1
+
+
+# Disallow --tag --check, with appropriate error
+returns_ 1 cksum --tag --check /dev/null 2> err
+grep 'the --tag option is meaningless when verifying checksums' err || fail=1
+# Allow --untagged --check, to better support
+# an emulation wrapper for legacy *sum utils.
+cksum -a md5 /dev/null | cksum --untagged --check || fail=1
+
 Exit $fail