Re: [PATCH 4/4] t4200: avoid passing a non-newline terminated file to sed

From: Brandon Casey <casey@nrlssc.navy.mil>
Date: May 6, 2009, 21:12 UTC
Message-ID: <RsLiW_EIDQ01u5uSMUrIIMzSbMhkfwGJBEGppONH79Im4WyT76bS5A@cipher.nrlssc.navy.mil>
In-Reply-To: <7vhbzyukyi.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 17 quoted lines

> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> Some versions of sed exit non-zero if the file they are supplied is not
>> newline terminated.  Solaris's /usr/xpg4/bin/sed is one such sed.  So
>> rework this test to avoid doing so.
> 
> I think up to your 3/4 is reasonable, but this is not enough for POSIX
> conformance (it is Ok if it is just aiming to fix "Solaris quirk").  POSIX
> sed is only required to work on text files, but .git/MERGE_RR is not a
> text file (it is a sequence of NUL terminated records).
> 
> I think something like this may work better.  Can somebody test?
> 
>> -	sha1=$(sed -e "s/	.*//" .git/MERGE_RR) &&
>> +	sha1=$({ cat .git/MERGE_RR; echo; } | sed -e "s/	.*//") &&
> 
> 	sha1=$(tr "\\000" "\\012" <./git/MERGE_RR | sed -e "s/	.*//") &&

I was about to reply that this fix works fine (actually, I was about to reply over an hour ago but was interrupted).

But, while testing it I noticed that you had a typo in your version that _did_not_ cause the test to fail. You have an extra slash in the path to '.git/MERGE_RR' which would have caused sha1 to be unset.

The 'sha1' variable that is set here on line 193 is used on the next line to set 'rr', but 'rr' is never used again. Unless I'm missing something, it appears these two lines can be deleted.

-brandon