[Top][All Lists]

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

Re: feedback on gzip 1.3.6

From: Paul Eggert
Subject: Re: feedback on gzip 1.3.6
Date: Sun, 26 Nov 2006 12:58:35 -0800
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)

Bdale Garbee <address@hidden> writes:

> The whole z* wrapper situation is a bit confusing to users at times,

No kidding.  I'd like to get rid of all the wrappers, by moving them
to another package, or something like that.  The wrappers have too
many bugs, as well as being confusing.

> Concensus among Debian users seems to be that if we really want a
> generic PAGER wrapper it should have a name like zpager.

I'd rather not add new wrappers until they're reorganized.  Until
then, I agree that zmore should probably not use PAGER, but I'd like
to defer this change to upstream until after the next stable release,
as it's not strictly a bug fix.

> There are a couple places in scripts where we're using symbols instead
> of numbers when setting traps.  I don't immediately remember why.

I saw those, and decided that they don't fix bugs that I can see, so I
left them out.  The change won't work on ancient shells that require
numbers instead of symbols for traps.  The change might be needed on
modern POSIX-conforming shells which don't support those old numbers,
which the standard allows, but I don't know of any such shells.

> Fixed a typo in docs since the last upload, gzip.1 has a 'ouput' that
> should be 'output'.

Yes, thanks, I have installed that in CVS gzip already.

> Some single vs double quoting around printfs in zgrep.in, not sure that
> matters.

It doesn't matter in standard-conforming shells, since '\n' and "\n"
have the same meaning in standard shells.  However, I think it safer
to use single-quoting, in case some nonstandard shell decides that
"\n" should stand for a newline character or something.

> There was some good reason for doing the sed to the argument name in
> zgrep.in that I can't recall offhand... hrm.

OK, thanks, I think I see the problem.  However, the patch isn't
correct: it mishandles file names containing newlines, and on many
hosts it'll also mishandle file names containing backslashes.  Also,
the patch assumes the newer $(...) construct, and I'd rather avoid
that if possible.  Instead, I installed the patch below into gzip CVS,
and it should appear in the next stable version.  (Now do you see why
I think those wrappers are a lost cause?  :-)

2006-11-26  Paul Eggert  <address@hidden>

        * zgrep.in: If the file name contains newline, &, \, or |, escape
        the character so that 'sed' doesn't mishandle it as a replacement.
        Problem reported by Bdale Garbee.

--- zgrep.in    20 Nov 2006 08:40:34 -0000      1.4
+++ zgrep.in    26 Nov 2006 20:47:27 -0000
@@ -116,10 +116,27 @@
     elif test $with_filename -eq 0 && { test $# -eq 1 || test $no_filename -eq 
1; }; then
       $grep $opt "$pat"
+      escaped=
+      while :; do
+       case $i in
+       *'
+         char='
+'        repl='\\n';;
+       *'&'*) char='&' repl='\&';;
+       *'\'*) char='\\' repl='\\';;
+       *'|'*) char='|' repl='\|';;
+       *) break;;
+       esac
+       up_to_first_char="\\([^$char]*\\)"
+       after_first_char="[^$char]*$char\\(.*\\)"
+       escaped=$escaped`expr "X$i" : "X$up_to_first_char"`$repl
+       i=`expr "X$i" : "$after_first_char"`
+      done
       if test $with_filename -eq 1; then
-       sed_script="s|^[^:]*:|${i}:|"
+       sed_script="s|[^:]*|$escaped$i|"
-       sed_script="s|^|${i}:|"
+       sed_script="s|^|$escaped$i:|"

       # Fail if either grep or sed fails.

reply via email to

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