bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug #19363] running any of the sum tools on a filename with gives s


From: Jim Meyering
Subject: Re: [bug #19363] running any of the sum tools on a filename with gives superfluous in output
Date: Wed, 21 Mar 2007 15:43:44 +0100

Eric Blake <address@hidden> wrote:
> According to Jim Meyering on 3/21/2007 3:52 AM:
>>>
>>> Closing.  The leading \ is indeed a feature, and has been documented in
>>> coreutils md5sum and sha*sum for years when \\ or \n appear in a filename
>
> It appears I am wrong.  I could find no mention of it in coreutils.texi.
>
>>> (although I would like to see \r added to this list).  There is no more
>>> elegant solution for handling arbitrary file names, and it is a security 
>>> hole
>>> if you can't handle arbitrary file names.
>>
>> Thanks for handling that.
>> I suppose the addition of \r-handling would be useful for Cygwin.
>> On other types of systems, file names containing \r are sufficiently
>> rare that the impact would be negligible.
>>
>> Do you want to add the feature?

Thanks for the quick patch.
At first glance, it looks very nice.
All it needs is an additional test case or two to exercise it.
No rush, though, since I'll put off applying it until after
the release.

> How about this?  A related change, but not added here until I get
> feedback, would be that if a line ends in \r, but a filename cannot be
> found that also ends in \r, then try stripping the \r rather than giving
> up right away.  In fact, if the \r escape sequence is seen, then always
> strip literal \r.  This would be more forgiving of the case where, even on
> Linux, a user is parsing a DOS-ending checksum file copied from a network.
>  Oh, and I guess I should submit a followup with testsuite changes, too
> (using tests/md5sum/newline-1 as a pattern).

As I see you realized :-)

> Also, I would really like to change the flag character for text vs. binary
> mode.  Ever since md5sum was changed to default to binary calculations
> (which was a good change, since you often run it on binaries, where a text
> sum is worthless), it is annoying that on cygwin, I have to manually
> delete the * flag character to make output match Linux.  I would much
> rather have an alternate flag character that is used to denote text mode,
> since I feel that is the exceptional case; but for backwards
> compatibility, it can't be *.  Would you accept a patch to make md5sum
> output ' ' for binary and '^' for text (instead of the current ' ' for
> text and '*' for binary), so long as it can continue to parse the old
> formats?  The only breakage will be people with a check file where
> checksums were created in text mode by older versions, but trying to
> verify using newer coreutils, and even then, it is only on platforms where
> text vs. binary makes a difference; nor is the breakage that common, since
> text-mode checksums have to be explicitly requested.

This does not sound unreasonable, but I'm reluctant to introduce
even such a small backwards-incompatibility just for systems (however
ubiquitous) that distinguish between binary and text files.
I'd like to hear if anyone else thinks this would be an acceptable change.

> ChangeLog:
> 2007-03-21  Eric Blake  <address@hidden>
>
>       Avoid ambiguity with checkfile in DOS text file format.
>       * src/md5sum.c (split_3): Parse escaped \r in filename.
>       (main): Escape \r on output.
>       * NEWS: Document this change.
>
> doc/ChangeLog:
> 2007-03-21  Eric Blake  <address@hidden>
>
>       * coreutils.texi (md5sum invocation): Document escapes in output
>       format.
>       Reported by Armijn Hemel.
>
> [Is there an easy way to make git or cogito commit the ChangeLog changes
> alongside the patch, but to omit them from the patchfile that I mail,
> since I am duplicating the changelog in the body of the mail?]

No.  It's true that ChangeLog handling is currently a pain.
Many projects using change-set-oriented version control systems
have just dropped ChangeLog files altogether, but I don't want
to do that.  At least not now.

For now, I'll apply the following and keep it on a branch.
Please send any subsequent patches relative to this one.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]