Subject: Re: [PATCH/RFC v1 1/1] test-lint: sed -E (or -a, -l) are not portable

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 16 Jan 2019 12:24:17 +0100

On Tue, Jan 15 2019, [email-masked] wrote:

> From: Torsten Bögershausen <[email-masked]>
>
> From `man sed` (on a Mac OS X box):
> The -E, -a and -i options are non-standard FreeBSD extensions and may not be available
> on other operating systems.
>
> From `man sed` on a Linux box:
> REGULAR EXPRESSIONS
>        POSIX.2 BREs should be supported, but they aren't completely because of
>        performance problems.  The \n sequence in a regular expression matches
>        the newline character,  and  similarly  for \a, \t, and other sequences.
>        The -E option switches to using extended regular expressions instead;
>        the -E option has been supported for years by GNU sed, and is now
>        included in POSIX.
>
> Well, there are still a lot of systems out there, which don't support it.
>
> Beside that, see IEEE Std 1003.1TM-2017
> http://pubs.opengroup.org/onlinepubs/9699919799/
> does not mention -E either.
>
> To be on the safe side, don't allow it.
>
> Reported-by: SZEDER Gábor <[email-masked]>
> Signed-off-by: Torsten Bögershausen <[email-masked]>
> ---
>
> I am somewhat unsure if we should disable all options except -e -f -n
> instead ?
> /\bsed\s+-[^efn]/ and err 'Not portable option with sed. Only -n -e -f are portable';
>
> That would cause a false positive in t9001 here:
> "--cc-cmd=./cccmd-sed --suppress-cc=self"
>
> which could either be fixed by an anchor:
> /^\s*sed\s+-[^efn]/
>
> Or by allowing '--' like this:
> /\bsed\s+-[^-efn]/
>
> Any thoughts, please ?
>
> t/check-non-portable-shell.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
> index b45bdac688..96b6afdeb8 100755
> --- a/t/check-non-portable-shell.pl
> +++ b/t/check-non-portable-shell.pl
> @@ -35,7 +35,7 @@ sub err {
>  		chomp;
>  	}
>
> -	/\bsed\s+-i/ and err 'sed -i is not portable';
> +	/\bsed\s+-[Eail]/ and err 'Not portable option with sed. Only -e -f -n are portable';
>  	/\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)';
>  	/^\s*declare\s+/ and err 'arrays/declare not portable';
>  	/^\s*[^#]\s*which\s/ and err 'which is not portable (use type)';

I'd just go for your /\bsed\s+-[^-efn]/ suggestion. Just a note if we do
go for the whitelist: According to GNU sed's manpage -E is also known as
-r, so /\bsed\s+-[Erail]/ would be better.