Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread jidanni
PS> Otherwise I'd like to hear the submitters opinion as well.

The submitter leaves it all in your hands as I am not as sharp as I
used to be. Thanks.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Jim Meyering
Hi again,

Patrick Schoenfeld <[EMAIL PROTECTED]> wrote:
> Hi Jim,
>
> On Fri, Nov 14, 2008 at 09:09:09PM +0100, Jim Meyering wrote:
>> If you're serious about it, and can sign a copyright assignment,
>> please look through the coreutils contribution/style guidelines
>>   http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=HACKING;hb=HEAD
>> and make the following changes/additions:
>
> I was a bit sceptic about this copyright assignment and wondered why the
> HACKING file does not include any link to [1]. Just as a suggestion

What's [1] ?

> cause I think some people might find uneasy about this ("what cow is it
> that I am buying?").
>
>>   - see if some other md5/md5sum program has a similar option
>
> I do not understand that. Which other programs do you mean?
> Utilities in other operating systems, e.g. BSDs md5sum or what do you
> mean? What rationale is behind this?

Exactly.  Do any of the NetBSD, FreeBSD, OpenBSD, Solaris, HPUX,
AIX, etc. programs (sometimes called "md5") have an option to do this?
If there *is* another program with this functionality, then you'd have a
good argument for adding a short-named option: to be compatible with it.
If not, then no new short-named options, on principle: there will be
less risk of conflict with other vendors or evolving/future standards.

>>   - follow instructions in HACKING regarding copyright assignment
>>   start this right away, since it'll take at least two weeks
>>   - make your changes relative to the latest in git (see HACKING)
>>   - add a NEWS entry
>>   - update doc/coreutils.texi
>
> OK. Will work on this (and the testcases) soon. Mail to assign@ has
> already been sent.

Great!

>>   - remove the -p short option
>
> Thats no problem, but just out of curiosity (and to avoid further
> mistakes) is there a specific reason for it? Do you fear that people
> could confuse it with 'preserve' option as seen on other tools, even
> though md5sum does not have something like this?

No. see above.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Jim Meyering
Patrick Schoenfeld <[EMAIL PROTECTED]> wrote:
> ehh.. forgotten to write about this one:
>
> On Fri, Nov 14, 2008 at 09:09:09PM +0100, Jim Meyering wrote:
>> - demonstrate --check --pedantic failing due to invalid first line
>> (ensure it still processes following ones -- should it warn
>> about each? or just the first?)
>
> I don't have a strong opinion about this, but I wonder which behaviour
> would be the best: Process every line without stopping on the first
> invalid line or Process every line until one invalid line is found (like
> it is implemented currently). From your sentence I guess you'd prefer
> the first? Is this a strong requirement in order for the patch to be
> included into coreutils? Otherwise I'd like to hear the submitters
> opinion as well.

It should process every line.
My question was whether to emit a diagnostic for each _malformed_ line,
or just for the first or maybe the first few.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread jidanni
JM> It should process every line.
JM> My question was whether to emit a diagnostic for each _malformed_ line,
JM> or just for the first or maybe the first few.

Like cmp(1) bail out if any trouble perhaps, but have options for fuller
digging/reports. OK, never mind... I leave it in your hands.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Jim Meyering
[EMAIL PROTECTED] wrote:

> JM> It should process every line.
> JM> My question was whether to emit a diagnostic for each _malformed_ line,
> JM> or just for the first or maybe the first few.
>
> Like cmp(1) bail out if any trouble perhaps, but have options for fuller
> digging/reports. OK, never mind... I leave it in your hands.

Whether you'd want to stop on the first "problem"
is orthogonal to this new --pedantic option.
This new option merely adjusts what is considered to be a problem.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Jim Meyering
Patrick Schoenfeld <[EMAIL PROTECTED]> wrote:
...
>> > I was a bit sceptic about this copyright assignment and wondered why the
>> > HACKING file does not include any link to [1]. Just as a suggestion
>>
>> What's [1] ?
>
> sorry, accidentally removed the link:
>
> http://www.gnu.org/licenses/why-assign.html

Good idea.
I've just added that link to HACKING.

  http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=8b09875b5

Normally I'd also add your name to the THANKS list,
but since you're preparing a patch, it'll soon appear
in a commit where you're the Author.

Which reminds me.
I intend to automatically generate the THANKS file from
the git logs, so if someone is listed (proper name and email from
most recent commit) as a commit Author:, then there will be no need
to manually add their name to the THANKS file.  Of course, there will
be a list of other name/email pairs, so the switch won't remove any
names.

>> > I do not understand that. Which other programs do you mean?
>> > Utilities in other operating systems, e.g. BSDs md5sum or what do you
>> > mean? What rationale is behind this?
>>
>> Exactly.  Do any of the NetBSD, FreeBSD, OpenBSD, Solaris, HPUX,
>> AIX, etc. programs (sometimes called "md5") have an option to do this?
>> If there *is* another program with this functionality, then you'd have a
>> good argument for adding a short-named option: to be compatible with it.
>> If not, then no new short-named options, on principle: there will be
>> less risk of conflict with other vendors or evolving/future standards.
>
> Makes sense. But this isn't a strong requirement is it? I don't even
> have access to all this operating systems ;)

With google, there's no need for access to the actual systems.
All I am looking for is assurance that you've performed a
duly-diligent search ;-)

> Hmpf. For now I'm fighting with the unreasonable dependencies of the
> coreutils git version.. automake 1.10a.. from the git repository.. =(

You can just change it to 1.10.1 for now.
[thought it might be easier just to build automake from scratch,
because, if you change it...]
You'll have to be careful not to submit that change as part of your patch.

Also, in your commit log message, be sure to attribute Dan and
to include the debian bug URL.

Thanks for doing all this,

Jim


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Patrick Schoenfeld

On Mon, Nov 17, 2008 at 11:25:09AM +0100, Jim Meyering wrote:
> Good idea.
> I've just added that link to HACKING.

Great :-)

> > Makes sense. But this isn't a strong requirement is it? I don't even
> > have access to all this operating systems ;)
> 
> With google, there's no need for access to the actual systems.
> All I am looking for is assurance that you've performed a
> duly-diligent search ;-)

True. Hm. Given that most utilities in the BSD world seem to use -p
different (while its notable that some of the utilities I looked at
don't even support checksum verification as part of their md5 utilities
but instead of another chksum utility) I see that your request to remove
-p is a valid objection.

Best Regards,
Patrick


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Patrick Schoenfeld
Hi Jim,

On Fri, Nov 14, 2008 at 09:09:09PM +0100, Jim Meyering wrote:
> If you're serious about it, and can sign a copyright assignment,
> please look through the coreutils contribution/style guidelines
>   http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=HACKING;hb=HEAD
> and make the following changes/additions:

I was a bit sceptic about this copyright assignment and wondered why the
HACKING file does not include any link to [1]. Just as a suggestion
cause I think some people might find uneasy about this ("what cow is it
that I am buying?").

>   - see if some other md5/md5sum program has a similar option

I do not understand that. Which other programs do you mean?
Utilities in other operating systems, e.g. BSDs md5sum or what do you
mean? What rationale is behind this?

>   - follow instructions in HACKING regarding copyright assignment
>   start this right away, since it'll take at least two weeks
>   - make your changes relative to the latest in git (see HACKING)
>   - add a NEWS entry
>   - update doc/coreutils.texi

OK. Will work on this (and the testcases) soon. Mail to assign@ has
already been sent.

>   - remove the -p short option

Thats no problem, but just out of curiosity (and to avoid further
mistakes) is there a specific reason for it? Do you fear that people
could confuse it with 'preserve' option as seen on other tools, even
though md5sum does not have something like this?

Best Regards,
Patrick


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Patrick Schoenfeld
Hi,

ehh.. forgotten to write about this one:

On Fri, Nov 14, 2008 at 09:09:09PM +0100, Jim Meyering wrote:
> - demonstrate --check --pedantic failing due to invalid first line
> (ensure it still processes following ones -- should it warn
> about each? or just the first?)

I don't have a strong opinion about this, but I wonder which behaviour
would be the best: Process every line without stopping on the first
invalid line or Process every line until one invalid line is found (like
it is implemented currently). From your sentence I guess you'd prefer
the first? Is this a strong requirement in order for the patch to be
included into coreutils? Otherwise I'd like to hear the submitters
opinion as well.

Best Regards,
Patrick


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#353911: md5sum --check checks only if _all_ are bad

2008-11-17 Thread Patrick Schoenfeld
Hi,

On Mon, Nov 17, 2008 at 10:34:02AM +0100, Jim Meyering wrote:
> Hi again,
> 
> Patrick Schoenfeld <[EMAIL PROTECTED]> wrote:
> > Hi Jim,
> >
> > On Fri, Nov 14, 2008 at 09:09:09PM +0100, Jim Meyering wrote:
> >> If you're serious about it, and can sign a copyright assignment,
> >> please look through the coreutils contribution/style guidelines
> >>   http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=HACKING;hb=HEAD
> >> and make the following changes/additions:
> >
> > I was a bit sceptic about this copyright assignment and wondered why the
> > HACKING file does not include any link to [1]. Just as a suggestion
> 
> What's [1] ?

sorry, accidentally removed the link:

http://www.gnu.org/licenses/why-assign.html


> > I do not understand that. Which other programs do you mean?
> > Utilities in other operating systems, e.g. BSDs md5sum or what do you
> > mean? What rationale is behind this?
> 
> Exactly.  Do any of the NetBSD, FreeBSD, OpenBSD, Solaris, HPUX,
> AIX, etc. programs (sometimes called "md5") have an option to do this?
> If there *is* another program with this functionality, then you'd have a
> good argument for adding a short-named option: to be compatible with it.
> If not, then no new short-named options, on principle: there will be
> less risk of conflict with other vendors or evolving/future standards.

Makes sense. But this isn't a strong requirement is it? I don't even
have access to all this operating systems ;)

Hmpf. For now I'm fighting with the unreasonable dependencies of the
coreutils git version.. automake 1.10a.. from the git repository.. =(

Regards,
Patrick


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


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

2008-11-17 Thread schoenfeld / in-medias-res
>From 156d7e829da3ab9d895a275c2cd02e52388bcd0d Mon Sep 17 00:00:00 2001
From: Patrick Schoenfeld <[EMAIL PROTECTED]>
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)

Signed-off-by: Patrick Schoenfeld <[EMAIL PROTECTED]>
---

The following patch misses tests. I would like to kindly ask if
someone else could add these. However I verified that these works:

Create testdata:
mkdir -p testdata
for i in $(seq 1 3); do echo $i > testdata/$i; done

md5sum testdata/*|sed '1s/^./Z/'|md5sum -c --pedantic -w
Expected Result: Warning about line 1, exit 1

md5sum testdata/*|sed '2s/^./Z/'|md5sum -c --pedantic -w
Expected Result: Warning about line 2, exit 1

md5sum testdata/*|sed 's/^./Z/'|md5sum -c --pedantic -w
Expected Result: Warn about all lines, moan that no properly formatted
line was found and exit 1.

md5sum testdata/*|md5sum -c --pedantic -w
Expected Result: All lines OK.

Please note that I'm still in process of copyright assignment.
Send a mail to [EMAIL PROTECTED] but got no reply yet.

 NEWS   |4 
 doc/coreutils.texi |6 ++
 src/md5sum.c   |   34 --
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index cbea67c..b0e2345 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,10 @@ GNU coreutils NEWS-*- 
outline -*-
 
   stat -f recognizes the Lustre file system type
 
+  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
 
   seq 9223372036854775807 9223372036854775808 now prints only two numbers
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 935129f..b9b629d 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -3484,6 +3484,12 @@ When verifying checksums, warn about improperly 
formatted MD5 checksum lines.
 This option is useful only if all but a few lines in the checked input
 are valid.
 
[EMAIL PROTECTED] --pedantic
[EMAIL PROTECTED] --pedantic
[EMAIL PROTECTED] verifying MD5 checksums
+When verifying checksums, fail if one or more improperly formatted MD5 checksum
+line is found.
+
 @end table
 
 @exitstatus
diff --git a/src/md5sum.c b/src/md5sum.c
index 969cc71..89ffd49 100644
--- a/src/md5sum.c
+++ b/src/md5sum.c
@@ -121,12 +121,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 pedantic = 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,
+  PEDANTIC_OPTION
 };
 
 static struct option const long_options[] =
@@ -137,6 +142,7 @@ static struct option const long_options[] =
   { "status", no_argument, NULL, STATUS_OPTION },
   { "text", no_argument, NULL, 't' },
   { "warn", no_argument, NULL, 'w' },
+  { "pedantic", no_argument, NULL, PEDANTIC_OPTION },
   { GETOPT_HELP_OPTION_DECL },
   { GETOPT_VERSION_OPTION_DECL },
   { NULL, 0, NULL, 0 }
@@ -184,6 +190,8 @@ The following three options are useful only when verifying 
checksums:\n\
   --quiet don't print OK for each successfully verified file\n\
   --statusdon't output anything, status code shows success\n\
   -w, --warn  warn about improperly formatted checksum lines\n\
+  --pedantic  return a non-zero exit code if one or more 
improperly\n\
+  formatted checksume line is found\n\
 \n\
 "), stdout);
   fputs (HELP_OPTION_DESCRIPTION, stdout);
@@ -428,6 +436,7 @@ digest_check (const char *checkfile_name)
 {
   FILE *checkfile_stream;
   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];
@@ -493,6 +502,8 @@ digest_check (const char *checkfile_name)
 checkfile_name, line_number,
 DIGEST_TYPE_STRING);
}
+
+   ++n_improperly_formatted_lines;
}
   else
{
@@ -591,8 +602,17 @@ digest_check (const char *checkfile_name)
 n_mismatched_checksums, n_computed_checksums);
}
}
-}
 
+  if (n_improperly_formatted_lines != 0)
+{
+ if (pedantic)
+   {
+   /* Bail out if