Subject: bug#7243: [patch] making md5sum's warnings clearer

From: Jim Meyering <[email-masked]>
Date: Tue, 19 Oct 2010 09:37:30 +0200

Benno Schulenberg wrote:
> Two years ago I complained about the untranslatability of the warnings
> that md5sum prints when it has encountered unexpected things:
> http://lists.gnu.org/archive/html/bug-coreutils/2008-09/msg00168.html
>
> Jim asked to come up with a change that doesn't remove information.
> I've finally gotten around to making this -- see the attached patch.
> In addition to read failures and checksum failures, md5sum now alerts
> the user to the number of misformatted checksum lines.  It prints this
> line also when '--warn' is not given, because it is so important.

Thanks for following up.
I've applied your patch after adjusting the log and a failing test,
then added two more tests to cover the new types of output lines.

>From 407806b27f032b3bce828ee0a152b5f67e997e09 Mon Sep 17 00:00:00 2001
From: Benno Schulenberg <[email-masked]>
Date: Mon, 18 Oct 2010 22:35:39 +0200
Subject: [PATCH 1/2] md5sum: print a summary warning for improperly formatted lines

And remove the now-superfluous totals from the other two warnings,
so the plurals will also work in other languages than English.

* src/md5sum.c (digest_check): Change as above.
* tests/misc/md5sum (check-quiet2): Adjust accordingly.
---
 src/md5sum.c      |   36 +++++++++++++++++++-----------------
 tests/misc/md5sum |    2 +-
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/md5sum.c b/src/md5sum.c
index 10d4fa2..a660e3b 100644
--- a/src/md5sum.c
+++ b/src/md5sum.c
@@ -431,6 +431,7 @@ static bool
 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_mismatched_checksums = 0;
   uintmax_t n_open_or_read_failures = 0;
@@ -489,6 +490,8 @@ digest_check (const char *checkfile_name)
              && ! (is_stdin && STREQ (filename, "-"))
              && hex_digits (hex_digest)))
         {
+          ++n_misformatted_lines;
+
           if (warn)
             {
               error (0, 0,
@@ -571,27 +574,26 @@ digest_check (const char *checkfile_name)
     {
       if (!status_only)
         {
+          if (n_misformatted_lines != 0)
+            error (0, 0,
+                   ngettext ("WARNING: %" PRIuMAX " line is improperly formatted",
+                             "WARNING: %" PRIuMAX " lines are improperly formatted",
+                             select_plural (n_misformatted_lines)),
+                   n_misformatted_lines);
+
           if (n_open_or_read_failures != 0)
             error (0, 0,
-                   ngettext ("WARNING: %" PRIuMAX " of %" PRIuMAX
-                             " listed file could not be read",
-                             "WARNING: %" PRIuMAX " of %" PRIuMAX
-                             " listed files could not be read",
-                             select_plural (n_properly_formatted_lines)),
-                   n_open_or_read_failures, n_properly_formatted_lines);
+                   ngettext ("WARNING: %" PRIuMAX " listed file could not be read",
+                             "WARNING: %" PRIuMAX " listed files could not be read",
+                             select_plural (n_open_or_read_failures)),
+                   n_open_or_read_failures);

           if (n_mismatched_checksums != 0)
-            {
-              uintmax_t n_computed_checksums =
-                (n_properly_formatted_lines - n_open_or_read_failures);
-              error (0, 0,
-                     ngettext ("WARNING: %" PRIuMAX " of %" PRIuMAX
-                               " computed checksum did NOT match",
-                               "WARNING: %" PRIuMAX " of %" PRIuMAX
-                               " computed checksums did NOT match",
-                               select_plural (n_computed_checksums)),
-                     n_mismatched_checksums, n_computed_checksums);
-            }
+            error (0, 0,
+                   ngettext ("WARNING: %" PRIuMAX " computed checksum did NOT match",
+                             "WARNING: %" PRIuMAX " computed checksums did NOT match",
+                             select_plural (n_mismatched_checksums)),
+                   n_mismatched_checksums);
         }
     }

diff --git a/tests/misc/md5sum b/tests/misc/md5sum
index d3c523e..92cc0fb 100755
--- a/tests/misc/md5sum
+++ b/tests/misc/md5sum
@@ -51,7 +51,7 @@ my @Tests =
      ['check-quiet2', '--check', '--quiet',
                                 {IN=>{'f.md5' => "$degenerate  f\n"}},
                                 {AUX=> {f=> 'foo'}}, {OUT=>"f: FAILED\n"},
-                                {ERR=>"md5sum: WARNING: 1 of 1 computed"
+                                {ERR=>"md5sum: WARNING: 1 computed"
                                        . " checksum did NOT match\n"},
                                 {EXIT=> 1}],
      # The sha1sum and md5sum drivers share a lot of code.
--
1.7.3.1.526.g2ee4

>From 10a0bdfa986dc77b9c5b7171b701e77d02756759 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Tue, 19 Oct 2010 09:19:59 +0200
Subject: [PATCH 2/2] tests: trigger and test for md5sum's new diagnostics

* tests/misc/md5sum: Test for new diagnostics.
---
 tests/misc/md5sum |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/tests/misc/md5sum b/tests/misc/md5sum
index 92cc0fb..819a819 100755
--- a/tests/misc/md5sum
+++ b/tests/misc/md5sum
@@ -54,6 +54,29 @@ my @Tests =
                                 {ERR=>"md5sum: WARNING: 1 computed"
                                        . " checksum did NOT match\n"},
                                 {EXIT=> 1}],
+     # Exercise new-after-8.6, easier-to-translate diagnostics.
+     ['check-multifail', '--check',
+                                {IN=>{'f.md5' =>
+                                      "$degenerate  f\n"
+                                      . "$degenerate  f\n"
+                                      . "invalid\n" }},
+                                {AUX=> {f=> 'foo'}},
+                                {OUT=>"f: FAILED\nf: FAILED\n"},
+                  {ERR=>"md5sum: WARNING: 1 line is improperly formatted\n"
+                      . "md5sum: WARNING: 2 computed checksums did NOT match\n"},
+                                {EXIT=> 1}],
+     # Similar to the above, but use --warn to evoke one more diagnostic.
+     ['check-multifail-warn', '--check', '--warn',
+                                {IN=>{'f.md5' =>
+                                      "$degenerate  f\n"
+                                      . "$degenerate  f\n"
+                                      . "invalid\n" }},
+                                {AUX=> {f=> 'foo'}},
+                                {OUT=>"f: FAILED\nf: FAILED\n"},
+              {ERR=>"md5sum: f.md5: 3: improperly formatted MD5 checksum line\n"
+                  . "md5sum: WARNING: 1 line is improperly formatted\n"
+                  . "md5sum: WARNING: 2 computed checksums did NOT match\n"},
+                                {EXIT=> 1}],
      # The sha1sum and md5sum drivers share a lot of code.
      # Ensure that md5sum does *not* share the part that makes
      # sha1sum accept BSD format.
--
1.7.3.1.526.g2ee4