Re: [PATCH] md5sum: Implemented --pedantic option to be more strict in verification mode

[Jim Meyering]

Jim Meyering wrote:

> Jim Meyering wrote:
>
>> schoenfeld / in-medias-res wrote:
>>>>From 156d7e829da3ab9d895a275c2cd02e52388bcd0d Mon Sep 17 00:00:00 2001
>>> From: Patrick Schoenfeld <address@hidden>
>>> Date: Mon, 17 Nov 2008 20:54:08 +0100
>>> Subject: [PATCH] md5sum: Implemented --pedantic option to be more strict in 
>>> verification mode
>>>
>>> * md5sum: Implemented a --pedantic option in --check mode, which lets md5sum
>>>   bail out with a non-zero exit code, if one or more improperly formatted 
>>> line
>>>   is found. Feature request by Dan Jacobson.
>>>   (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=353911)
>> ...
>>> Please note that I'm still in process of copyright assignment.
>>> Send a mail to assign@... but got no reply yet.
>> ...
>>
>>> + md5sum now recognizes a new option --pedantic when verifying
>>> md5sums in a file
>>> +  (with --check) to let it return a non-zero exit code if one or more 
>>> invalid
>>> +  lines are found
>>> +
>>>  ** Bug fixes
>>
>> Thanks for the patch.
>> Your copyright assignment did go through.
>>
>> I've rebased your patch and made a few minor changes (NEWS, --help,
>> indentation).
>> More will be required:
>>   - also describe the option in sections for sha1sum and the other
>>     sha*sum programs; probably use a macro to avoid duplication
>
> It turns out this is not necessary.
> The sha* section already defers to the md5sum description
> saying they have exactly the same options.
>
>>   - when checking two or more files, don't stop processing (exit) upon
>>     an invalid line in the first.  i.e., process all files before exiting.
>
> This was trivial to adjust.
>
>>   - decide on the option name: I prefer --strict;  --pedantic comes
>>     with connotations that seem unjustified.
>
> I made that change
>
>>   - exercise all of this via the test suite
>
> Added below.
> I'll squash the first three commits into Patrick's commit.
...
> Subject: [PATCH 1/4] don't exit on first losing file

In case anyone wants to review, here's what I expect to
push later today:

>From f1f10db5dbff744f96917ff87956b90e21afb44b Mon Sep 17 00:00:00 2001
From: Patrick Schoenfeld <address@hidden>
Date: Thu, 7 Jul 2011 08:57:39 +0200
Subject: [PATCH 1/2] md5sum, sha1sum, etc: accept new option: --strict

Use this new option with --check when the input is expected to
consist solely of checksum lines.  With only --check, an invalid
line evokes a warning, but the program can still exit successfully.
With --strict, any invalid line makes the program exit non-zero.

* src/md5sum.c (strict, STRICT_OPTION): Declare/define.
(long_options): Add "strict".
(usage): Describe --strict.
(digest_check): Count improperly_formatted lines, too, and use
that number and the global "strict" to determine the return value.
(main): Handle STRICT_OPTION.
Reject --strict without --check.
* doc/coreutils.texi: Describe it.
* NEWS (New features): Mention it.
---
 NEWS               |    4 ++++
 doc/coreutils.texi |    7 +++++++
 src/md5sum.c       |   27 +++++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index f7e7823..32dab7a 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   Note the use of single quotes, not double quotes.
   That creates files named xaa.xz, xab.xz and xac.xz.

+  md5sum accepts the new --strict option.  With --check, it makes the
+  tool exit non-zero for any invalid input line, rather than just warning.
+  This also affects sha1sum, sha224sum, sha384sum and sha512sum.
+
 ** Improvements

   shuf outputs small subsets of large permutations much more efficiently.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index c59af2f..95cc44a 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -3702,6 +3702,13 @@ md5sum invocation
 This option is useful only if all but a few lines in the checked input
 are valid.

+@itemx --strict
+@opindex --strict
+@cindex verifying MD5 checksums
+When verifying checksums,
+if one or more input line is invalid,
+exit nonzero after all warnings have been issued.
+
 @end table

 @exitstatus
diff --git a/src/md5sum.c b/src/md5sum.c
index 9bbdc60..ff9538a 100644
--- a/src/md5sum.c
+++ b/src/md5sum.c
@@ -122,12 +122,17 @@ static bool warn = false;
 /* With --check, suppress the "OK" printed for each verified file.  */
 static bool quiet = false;

+/* With --check, exit with a non-zero return code if any line is
+   improperly formatted. */
+static bool strict = false;
+
 /* For long options that have no equivalent short option, use a
    non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
 enum
 {
   STATUS_OPTION = CHAR_MAX + 1,
-  QUIET_OPTION
+  QUIET_OPTION,
+  STRICT_OPTION
 };

 static struct option const long_options[] =
@@ -138,6 +143,7 @@ static struct option const long_options[] =
   { "status", no_argument, NULL, STATUS_OPTION },
   { "text", no_argument, NULL, 't' },
   { "warn", no_argument, NULL, 'w' },
+  { "strict", no_argument, NULL, STRICT_OPTION },
   { GETOPT_HELP_OPTION_DECL },
   { GETOPT_VERSION_OPTION_DECL },
   { NULL, 0, NULL, 0 }
@@ -187,6 +193,9 @@ The following three options are useful only when verifying 
checksums:\n\
   -w, --warn           warn about improperly formatted checksum lines\n\
 \n\
 "), stdout);
+      fputs (_("\
+      --strict         with --check, exit non-zero for any invalid input\n\
+"), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
       printf (_("\
@@ -434,6 +443,7 @@ digest_check (const char *checkfile_name)
   FILE *checkfile_stream;
   uintmax_t n_misformatted_lines = 0;
   uintmax_t n_properly_formatted_lines = 0;
+  uintmax_t n_improperly_formatted_lines = 0;
   uintmax_t n_mismatched_checksums = 0;
   uintmax_t n_open_or_read_failures = 0;
   unsigned char bin_buffer_unaligned[DIGEST_BIN_BYTES + DIGEST_ALIGN];
@@ -501,6 +511,8 @@ digest_check (const char *checkfile_name)
                      checkfile_name, line_number,
                      DIGEST_TYPE_STRING);
             }
+
+          ++n_improperly_formatted_lines;
         }
       else
         {
@@ -603,7 +615,8 @@ digest_check (const char *checkfile_name)

   return (n_properly_formatted_lines != 0
           && n_mismatched_checksums == 0
-          && n_open_or_read_failures == 0);
+          && n_open_or_read_failures == 0
+          && (!strict || n_improperly_formatted_lines == 0));
 }

 int
@@ -657,6 +670,9 @@ main (int argc, char **argv)
         warn = false;
         quiet = true;
         break;
+      case STRICT_OPTION:
+        strict = true;
+        break;
       case_GETOPT_HELP_CHAR;
       case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
       default:
@@ -694,6 +710,13 @@ main (int argc, char **argv)
       usage (EXIT_FAILURE);
     }

+  if (strict & !do_check)
+   {
+     error (0, 0,
+        _("the --strict option is meaningful only when verifying checksums"));
+     usage (EXIT_FAILURE);
+   }
+
   if (!O_BINARY && binary < 0)
     binary = 0;

--
1.7.6.430.g34be2


>From 594df76d66824ab48de165db8eb151d5d66892dc Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 7 Jul 2011 12:12:40 +0200
Subject: [PATCH 2/2] tests: exercise md5sum's new --strict option

* tests/misc/md5sum: Exercise new --strict option.
---
 tests/misc/md5sum |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/tests/misc/md5sum b/tests/misc/md5sum
index f09a200..e8877e0 100755
--- a/tests/misc/md5sum
+++ b/tests/misc/md5sum
@@ -43,6 +43,23 @@ my @Tests =
      ['check-1', '--check', {AUX=> {f=> ''}},
                                 {IN=> {'f.md5' => "$degenerate  f\n"}},
                                 {OUT=>"f: OK\n"}],
+
+     # Same as above, but with an added empty line, to provoke --strict.
+     ['ck-strict-1', '--check --strict', {AUX=> {f=> ''}},
+                                {IN=> {'f.md5' => "$degenerate  f\n\n"}},
+                                {OUT=>"f: OK\n"},
+                                {ERR=>"md5sum: "
+                                 . "WARNING: 1 line is improperly 
formatted\n"},
+                                {EXIT=> 1}],
+
+     # As above, but with the invalid line first, to ensure that following
+     # lines are processed in spite of the preceding invalid input line.
+     ['ck-strict-2', '--check --strict', {AUX=> {f=> ''}},
+                                {IN=> {'in.md5' => "\n$degenerate  f\n"}},
+                                {OUT=>"f: OK\n"},
+                                {ERR=>"md5sum: "
+                                 . "WARNING: 1 line is improperly 
formatted\n"},
+                                {EXIT=> 1}],
      ['check-2', '--check', '--status', {IN=>{'f.md5' => "$degenerate  f\n"}},
                                 {AUX=> {f=> 'foo'}}, {EXIT=> 1}],
      ['check-quiet1', '--check', '--quiet', {AUX=> {f=> ''}},
--
1.7.6.430.g34be2