Re: [PATCH] Improve sed portability

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

 



Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:

> Chris Ridd schrieb:
>> On Solaris /usr/bin/sed apparently fails to process input that doesn't
>> end in a \n. Consequently constructs like
>> 
>>   re=$(printf '%s' foo | sed -e 's/bar/BAR/g' $)
>> 
>> cause re to be set to the empty string.
>
> So does /usr/bin/sed of AIX 4.3!
>
>> @@ -73,7 +73,7 @@ resolve_relative_url ()
>>  module_name()
>>  {
>>  	# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
>> -	re=$(printf '%s' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
>> +	re=$(printf "%s\n" "$1" | sed -e 's/[].[^$\\*]/\\&/g')
>
> You change sq into dq. Is this not dangerous? Shouldn't backslash-en be
> hidden from the shell so that printf can interpret it?

"\n" inside dq is _not_ interpreted by the shell (printf interprets it),
but I tend to agree that using sq is worry-free and better.

>>  	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
>>  		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
>
> I trust you have tested this. But I wonder whether this leaves a stray
> newline in $re that gets in the way inside the sed expression...

I suspect the very original was written (or copied from something that
wrote) like this:

	re=$(echo -n "$1" | sed -e '...')

and mechanically replaced to

	re=$(printf '%s' "$1" | sed -e '...')

because "echo" is not quite portable.

But the original misunderstands the command substitution.  The trailing LF
is removed by it, so as long as "$1" is a single line, $re will get a line
without the trailing LF _anyway_.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux