Re: [PATCH] git-am: force egrep to use correct characters set

[Johannes Sixt <j.sixt@viscovery.net>]

Christian Himpel schrieb:
> After doing a quick:
> 
> LC_ALL=C find . -name '*.sh' -exec \
>          egrep -Hne '(grep|awk|sed).*\[.*-.*\]' {} \;
> 
> As far as I can see, range expressions are used:

0. The test suite scripts are not critical because test-lib.sh sets LANG=C
LC_ALL=C

> 1. to replace or grep hexadecimal numbers (SHA1 sums).  This shouldn't
> be a problem, if we can assume that these numbers are never malformed.

The assumption is valid if the input is piped from a git command, and such
uses aren't critical.

> 2. to replace or grep numbers (with digits).  This shouldn't be a
> problem, since digits should be in dictionary order in every language
> (?!).

I agree.

> 3. in git-rebase--interactive.sh:742 to grep for a previously generated
> string.  So it should be safe here.

I agree.

>> The case identified by Christian is certainly important because it is
>> applied to a file whose contents can be anything, and the purpose of the
>> check is to identify the text as an mbox file, whose header section can be
>> only US-ASCII by definition. So, I think it has merit to apply the patch.
> 
> Yes.  It seems that this is the only place where it is important to match
> just the ASCII printable characters.

There is another place in git-am.sh where a sed expression with a range
looks at the input file, doesn't it? Isn't it critical, too?

		if test -f "$dotest/rebasing" &&
			commit=$(sed -e 's/^From \([0-9a-f]*\) .*/\1/' \
				-e q "$dotest/$msgnum") &&
			test "$(git cat-file -t "$commit")" = commit
		then ...

-- Hannes