[PATCH] tools: fix == bashism in kernel-chktaint

[PATCH] tools: fix == bashism in kernel-chktaint

[Kevin Locke]

When /bin/sh is a shell other than bash, invoking kernel-chktaint with
at least one argument may produce error messages such as the following
(produced by [dash] with argument 1024):

    ./kernel-chktaint: 22: [: 1024x: unexpected operator
    ./kernel-chktaint: 22: [: 1024x: unexpected operator

This occurs because the == operator is not specified for [test in POSIX]
and is not supported by all shells, as noted by shellcheck [SC3014].

To fix the issue and avoid the error message, replace == with =.

[dash]: https://git.kernel.org/pub/scm/utils/dash/dash.git
[test in POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html
[SC3014]: https://www.shellcheck.net/wiki/SC3014

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/debugging/kernel-chktaint | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
index e7da0909d0970..051608a63d9f1 100755
--- a/tools/debugging/kernel-chktaint
+++ b/tools/debugging/kernel-chktaint
@@ -19,7 +19,7 @@ EOF
 }
 
 if [ "$1"x != "x" ]; then
-	if  [ "$1"x == "--helpx" ] || [ "$1"x == "-hx" ] ; then
+	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
 		usage
 		exit 1
 	elif  [ $1 -ge 0 ] 2>/dev/null ; then
-- 
2.51.0

Re: [PATCH] tools: fix == bashism in kernel-chktaint

[Randy Dunlap]

On 10/11/25 2:04 PM, Kevin Locke wrote:
> When /bin/sh is a shell other than bash, invoking kernel-chktaint with
> at least one argument may produce error messages such as the following
> (produced by [dash] with argument 1024):
> 
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
> 
> This occurs because the == operator is not specified for [test in POSIX]
> and is not supported by all shells, as noted by shellcheck [SC3014].
> 
> To fix the issue and avoid the error message, replace == with =.
> 
> [dash]: https://git.kernel.org/pub/scm/utils/dash/dash.git
> [test in POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html
> [SC3014]: https://www.shellcheck.net/wiki/SC3014
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  tools/debugging/kernel-chktaint | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index e7da0909d0970..051608a63d9f1 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -19,7 +19,7 @@ EOF
>  }
>  
>  if [ "$1"x != "x" ]; then
> -	if  [ "$1"x == "--helpx" ] || [ "$1"x == "-hx" ] ; then
> +	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
>  		usage
>  		exit 1
>  	elif  [ $1 -ge 0 ] 2>/dev/null ; then

-- 
~Randy

Re: [PATCH] tools: fix == bashism in kernel-chktaint

[Randy Dunlap]

On 10/11/25 3:07 PM, Randy Dunlap wrote:
> 
> 
> On 10/11/25 2:04 PM, Kevin Locke wrote:
>> When /bin/sh is a shell other than bash, invoking kernel-chktaint with
>> at least one argument may produce error messages such as the following
>> (produced by [dash] with argument 1024):
>>
>>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
>>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
>>
>> This occurs because the == operator is not specified for [test in POSIX]
>> and is not supported by all shells, as noted by shellcheck [SC3014].
>>
>> To fix the issue and avoid the error message, replace == with =.
>>
>> [dash]: https://git.kernel.org/pub/scm/utils/dash/dash.git
>> [test in POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html
>> [SC3014]: https://www.shellcheck.net/wiki/SC3014
>>
>> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> 
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

and
Tested-by: Randy Dunlap <rdunlap@infradead.org>

> Thanks.
> 
>> ---
>>  tools/debugging/kernel-chktaint | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
>> index e7da0909d0970..051608a63d9f1 100755
>> --- a/tools/debugging/kernel-chktaint
>> +++ b/tools/debugging/kernel-chktaint
>> @@ -19,7 +19,7 @@ EOF
>>  }
>>  
>>  if [ "$1"x != "x" ]; then
>> -	if  [ "$1"x == "--helpx" ] || [ "$1"x == "-hx" ] ; then
>> +	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
>>  		usage
>>  		exit 1
>>  	elif  [ $1 -ge 0 ] 2>/dev/null ; then
> 

-- 
~Randy

Re: [PATCH] tools: fix == bashism in kernel-chktaint

[Thorsten Leemhuis]

On 10/11/25 23:04, Kevin Locke wrote:
> When /bin/sh is a shell other than bash, invoking kernel-chktaint with
> at least one argument may produce error messages such as the following
> (produced by [dash] with argument 1024):
> 
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
> 
> This occurs because the == operator is not specified for [test in POSIX]
> and is not supported by all shells, as noted by shellcheck [SC3014].
> 
> To fix the issue and avoid the error message, replace == with =.

Many thx for this!

Acked-by: Thorsten Leemhuis <linux@leemhuis.info>

BTW, I doubt that anyone cares, but I wonder if these are really necessary:
> [dash]: https://git.kernel.org/pub/scm/utils/dash/dash.git
> [test in POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html
> [SC3014]: https://www.shellcheck.net/wiki/SC3014
And maybe they should be proper link tags (with references at the end),
as mentioned in
https://www.kernel.org/doc/html/latest/process/5.Posting.html (search
for "optional-other-stuff"). But overall I guess those links are not
really worth it.

Ciao, Thorsten

[PATCH v2] tools: fix == bashism in kernel-chktaint

[Kevin Locke]

When /bin/sh is a shell other than bash, invoking kernel-chktaint with
at least one argument may produce error messages such as the following
(produced by dash 0.5.12 with argument 1024):

    ./kernel-chktaint: 22: [: 1024x: unexpected operator
    ./kernel-chktaint: 22: [: 1024x: unexpected operator

This occurs because the == operator is not specified for test in POSIX
and is not supported by all shells, as noted by shellcheck SC3014.

To fix the issue and avoid the error message, replace == with =.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---

Thanks for the reviews and feedback Thorsten and Randy!  I agree it
probably doesn't matter, but I appreciate the value of consistency and
of being aware of those conventions, and it's an easy fix.  Thanks for
letting me know about it!  Here's an updated version with an improved
commit message.

Thanks again,
Kevin

Changes since v1:
 * Remove superfluous links in Markdown format from commit message.
 * Add version of dash used to commit message.

 tools/debugging/kernel-chktaint | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
index e7da0909d0970..051608a63d9f1 100755
--- a/tools/debugging/kernel-chktaint
+++ b/tools/debugging/kernel-chktaint
@@ -19,7 +19,7 @@ EOF
 }
 
 if [ "$1"x != "x" ]; then
-	if  [ "$1"x == "--helpx" ] || [ "$1"x == "-hx" ] ; then
+	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
 		usage
 		exit 1
 	elif  [ $1 -ge 0 ] 2>/dev/null ; then
-- 
2.51.0

Re: [PATCH v2] tools: fix == bashism in kernel-chktaint

[Randy Dunlap]

On 10/13/25 7:41 AM, Kevin Locke wrote:
> When /bin/sh is a shell other than bash, invoking kernel-chktaint with
> at least one argument may produce error messages such as the following
> (produced by dash 0.5.12 with argument 1024):
> 
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
> 
> This occurs because the == operator is not specified for test in POSIX
> and is not supported by all shells, as noted by shellcheck SC3014.
> 
> To fix the issue and avoid the error message, replace == with =.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

This should have retained the previous *-by: messages.

> ---
> 
> Thanks for the reviews and feedback Thorsten and Randy!  I agree it
> probably doesn't matter, but I appreciate the value of consistency and
> of being aware of those conventions, and it's an easy fix.  Thanks for
> letting me know about it!  Here's an updated version with an improved
> commit message.
> 
> Thanks again,
> Kevin
> 
> Changes since v1:
>  * Remove superfluous links in Markdown format from commit message.
>  * Add version of dash used to commit message.
> 
>  tools/debugging/kernel-chktaint | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index e7da0909d0970..051608a63d9f1 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -19,7 +19,7 @@ EOF
>  }
>  
>  if [ "$1"x != "x" ]; then
> -	if  [ "$1"x == "--helpx" ] || [ "$1"x == "-hx" ] ; then
> +	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
>  		usage
>  		exit 1
>  	elif  [ $1 -ge 0 ] 2>/dev/null ; then

-- 
~Randy

Re: [PATCH] tools: fix == bashism in kernel-chktaint

[David Laight]

On Sat, 11 Oct 2025 15:04:26 -0600
Kevin Locke <kevin@kevinlocke.name> wrote:

> When /bin/sh is a shell other than bash, invoking kernel-chktaint with
> at least one argument may produce error messages such as the following
> (produced by [dash] with argument 1024):
> 
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
>     ./kernel-chktaint: 22: [: 1024x: unexpected operator
> 
> This occurs because the == operator is not specified for [test in POSIX]
> and is not supported by all shells, as noted by shellcheck [SC3014].
> 
> To fix the issue and avoid the error message, replace == with =.
> 
> [dash]: https://git.kernel.org/pub/scm/utils/dash/dash.git
> [test in POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html
> [SC3014]: https://www.shellcheck.net/wiki/SC3014
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---
>  tools/debugging/kernel-chktaint | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index e7da0909d0970..051608a63d9f1 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -19,7 +19,7 @@ EOF
>  }
>  
>  if [ "$1"x != "x" ]; then
> -	if  [ "$1"x == "--helpx" ] || [ "$1"x == "-hx" ] ; then
> +	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then

Ugg - one of the reasons for adding an x is to stop the string being treated as
an operator - but you need to to add at the front, not the end.
You don't need one to avoid an empty string - the quotes to that.

Although, IIRC, the posix standard requires the 3-operand expressions
be evaluated 'as expected' even if the first is (say) "-n".
ISTR that the 5-operand [ a = b -o c = d ] is also required to be parsed.
But, in any case, you can do:
	if [ "x$1" = x--help -o "x$1" = x-h ]; then

  David


>  		usage
>  		exit 1
>  	elif  [ $1 -ge 0 ] 2>/dev/null ; then

[PATCH] tools: remove unnecessary x suffix in test strings

[Kevin Locke]

An "x" suffix was appended to test variable expansions, presumably to
avoid issues with empty strings in some old shells, or perhaps with the
intention of avoiding issues with dashes or other special characters
that an "x" prefix might have avoided.  In either case, POSIX ensures
that such protections are not necessary, and are unlikely to be
encountered in shells currently in use, as indicated by shellcheck
SC2268.

Remove the "x" suffixes which unnecessarily complicate the code.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Suggested-by: David Laight <david.laight.linux@gmail.com>
---

Thanks David, that's a good point about the x suffixes.  Since
shellcheck warns about the x prefixes (SC2268) and I'm not aware of any
shells currently in use which require them, I think they are safe to
remove to clean up the code a bit.  Here's a patch to do just that,
which can be applied on top of my previous patch.

Since -o is an XSI extension to POSIX, I've stuck with ||, but I think
you are right that x would not be required in that case either.

Thanks again,
Kevin


 tools/debugging/kernel-chktaint | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
index 051608a63d9f..051ac27b58eb 100755
--- a/tools/debugging/kernel-chktaint
+++ b/tools/debugging/kernel-chktaint
@@ -18,8 +18,8 @@ retrieved from /proc/sys/kernel/tainted on another system.
 EOF
 }
 
-if [ "$1"x != "x" ]; then
-	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
+if [ "$1" != "" ]; then
+	if  [ "$1" = "--help" ] || [ "$1" = "-h" ] ; then
 		usage
 		exit 1
 	elif  [ $1 -ge 0 ] 2>/dev/null ; then
-- 
2.51.0

Re: [PATCH] tools: remove unnecessary x suffix in test strings

[Randy Dunlap]

On 10/16/25 4:47 PM, Kevin Locke wrote:
> An "x" suffix was appended to test variable expansions, presumably to
> avoid issues with empty strings in some old shells, or perhaps with the
> intention of avoiding issues with dashes or other special characters
> that an "x" prefix might have avoided.  In either case, POSIX ensures
> that such protections are not necessary, and are unlikely to be
> encountered in shells currently in use, as indicated by shellcheck
> SC2268.
> 
> Remove the "x" suffixes which unnecessarily complicate the code.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Suggested-by: David Laight <david.laight.linux@gmail.com>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
> 
> Thanks David, that's a good point about the x suffixes.  Since
> shellcheck warns about the x prefixes (SC2268) and I'm not aware of any
> shells currently in use which require them, I think they are safe to
> remove to clean up the code a bit.  Here's a patch to do just that,
> which can be applied on top of my previous patch.
> 
> Since -o is an XSI extension to POSIX, I've stuck with ||, but I think
> you are right that x would not be required in that case either.
> 
> Thanks again,
> Kevin
> 
> 
>  tools/debugging/kernel-chktaint | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index 051608a63d9f..051ac27b58eb 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -18,8 +18,8 @@ retrieved from /proc/sys/kernel/tainted on another system.
>  EOF
>  }
>  
> -if [ "$1"x != "x" ]; then
> -	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
> +if [ "$1" != "" ]; then
> +	if  [ "$1" = "--help" ] || [ "$1" = "-h" ] ; then
>  		usage
>  		exit 1
>  	elif  [ $1 -ge 0 ] 2>/dev/null ; then

-- 
~Randy

Re: [PATCH] tools: remove unnecessary x suffix in test strings

[David Laight]

On Thu, 16 Oct 2025 17:47:09 -0600
Kevin Locke <kevin@kevinlocke.name> wrote:

> An "x" suffix was appended to test variable expansions, presumably to
> avoid issues with empty strings in some old shells, or perhaps with the
> intention of avoiding issues with dashes or other special characters
> that an "x" prefix might have avoided.  In either case, POSIX ensures
> that such protections are not necessary, and are unlikely to be
> encountered in shells currently in use, as indicated by shellcheck
> SC2268.
> 
> Remove the "x" suffixes which unnecessarily complicate the code.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> ---
> 
> Thanks David, that's a good point about the x suffixes.  Since
> shellcheck warns about the x prefixes (SC2268) and I'm not aware of any
> shells currently in use which require them,

The problems arise when $1 is (say) "-x", a simple LR parser will treat
[ -x = -x ] as a check for the file "=" being executable and then give
a syntax error for the second -x.
I can't imagine why shellcheck should warn about a leading x (or any other
character) provided field splitting is disabled (eg by "").
The leading x has definitely been needed in the past.

POSIX does require the three argument 'test' look for the middle argument
being an operator - but there might be historic shells that don't so that.
OTOH you are probably looking for code from the early 1980s!
But the POSIX spec (last time I read it) does point out the problems
with arbitrary strings being treated as operators causing complex expressions
be mis-parsed - which a leading x fixes.

> I think they are safe to
> remove to clean up the code a bit.  Here's a patch to do just that,
> which can be applied on top of my previous patch.
> 
> Since -o is an XSI extension to POSIX, I've stuck with ||, but I think
> you are right that x would not be required in that case either.

I'm not sure there are any common shells that don't support -o and -a.
They get used quite a lot.
I'm pretty sure they were supported by the pre-POSIX System-V shells
(or the /bin/[ program they ran).

	David

> 
> Thanks again,
> Kevin
> 
> 
>  tools/debugging/kernel-chktaint | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index 051608a63d9f..051ac27b58eb 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -18,8 +18,8 @@ retrieved from /proc/sys/kernel/tainted on another system.
>  EOF
>  }
>  
> -if [ "$1"x != "x" ]; then
> -	if  [ "$1"x = "--helpx" ] || [ "$1"x = "-hx" ] ; then
> +if [ "$1" != "" ]; then
> +	if  [ "$1" = "--help" ] || [ "$1" = "-h" ] ; then
>  		usage
>  		exit 1
>  	elif  [ $1 -ge 0 ] 2>/dev/null ; then

Re: [PATCH] tools: remove unnecessary x suffix in test strings

[Kevin Locke]

On Fri, 2025-10-17 at 15:12 +0100, David Laight wrote:
> On Thu, 16 Oct 2025 17:47:09 -0600 Kevin Locke <kevin@kevinlocke.name> wrote:
>> Remove the "x" suffixes which unnecessarily complicate the code.
> 
> The problems arise when $1 is (say) "-x", a simple LR parser will treat
> [ -x = -x ] as a check for the file "=" being executable and then give
> a syntax error for the second -x.
> I can't imagine why shellcheck should warn about a leading x (or any other
> character) provided field splitting is disabled (eg by "").
> The leading x has definitely been needed in the past.

Yep, it definitely has been.  The rationale on the wiki is that it's
not necessary for modern shells (and presumably that it unnecessarily
complicates the code): https://www.shellcheck.net/wiki/SC2268
However, it notes Zsh had issues as recently as 2015, which is not as
old as I would have expected.

> POSIX does require the three argument 'test' look for the middle argument
> being an operator - but there might be historic shells that don't so that.
> OTOH you are probably looking for code from the early 1980s!
> But the POSIX spec (last time I read it) does point out the problems
> with arbitrary strings being treated as operators causing complex expressions
> be mis-parsed - which a leading x fixes.

Good point.  I just reread it and can confirm that the current version
still notes issues mitigated by the X prefix with "historical shells"
and with greater than 4 argument cases:
https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html

>> I think they are safe to
>> remove to clean up the code a bit.  Here's a patch to do just that,
>> which can be applied on top of my previous patch.
>> 
>> Since -o is an XSI extension to POSIX, I've stuck with ||, but I think
>> you are right that x would not be required in that case either.
> 
> I'm not sure there are any common shells that don't support -o and -a.
> They get used quite a lot.
> I'm pretty sure they were supported by the pre-POSIX System-V shells
> (or the /bin/[ program they ran).

You are probably right.  I still remember when Debian policy allowed
them and posh added support in 2007/2008:
https://lists.debian.org/debian-devel/2006/11/msg00710.html
(I was corrected by Clint Adams about -a and -o being XSI extensions
some years before then when I noted posh lacked support, which is
probably why I still remember it.)

I find && and || more readable, but I'm open to changing it if you
feel strongly.

Do I understand correctly that you are in favor of using the x prefix?
I have a slight preference for leaving it off, but I'm open to adding
it if you (or others) feel strongly.

Thanks for the interesting discussion,
Kevin

Re: [PATCH] tools: remove unnecessary x suffix in test strings

[David Laight]

On Fri, 17 Oct 2025 16:28:12 -0600
Kevin Locke <kevin@kevinlocke.name> wrote:

> On Fri, 2025-10-17 at 15:12 +0100, David Laight wrote:
> > On Thu, 16 Oct 2025 17:47:09 -0600 Kevin Locke <kevin@kevinlocke.name> wrote:  
> >> Remove the "x" suffixes which unnecessarily complicate the code.  
> > 
> > The problems arise when $1 is (say) "-x", a simple LR parser will treat
> > [ -x = -x ] as a check for the file "=" being executable and then give
> > a syntax error for the second -x.
> > I can't imagine why shellcheck should warn about a leading x (or any other
> > character) provided field splitting is disabled (eg by "").
> > The leading x has definitely been needed in the past.  
> 
> Yep, it definitely has been.  The rationale on the wiki is that it's
> not necessary for modern shells (and presumably that it unnecessarily
> complicates the code): https://www.shellcheck.net/wiki/SC2268
> However, it notes Zsh had issues as recently as 2015, which is not as
> old as I would have expected.

It doesn't really make much difference to the shell.
I really doubt you'll notice any difference in the time it takes to run.

> 
> > POSIX does require the three argument 'test' look for the middle argument
> > being an operator - but there might be historic shells that don't so that.
> > OTOH you are probably looking for code from the early 1980s!
> > But the POSIX spec (last time I read it) does point out the problems
> > with arbitrary strings being treated as operators causing complex expressions
> > be mis-parsed - which a leading x fixes.  
> 
> Good point.  I just reread it and can confirm that the current version
> still notes issues mitigated by the X prefix with "historical shells"
> and with greater than 4 argument cases:
> https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html

The fact that the 'greater than 4 argument case' can still require
a prefix character might be considered enough to make adding one all the
time 'good practise' even though it (probably) isn't actually needed.

While I wouldn't error not having a prefix, generating an error when
there is one seems wrong.

What does shellcheck do with [ "$a" = "$b" -o "$c" = "$d" ] ?
Or even [ "$a" "$b" "$c" "$d" "$e" "$f "$g" ] ??

> 
> >> I think they are safe to
> >> remove to clean up the code a bit.  Here's a patch to do just that,
> >> which can be applied on top of my previous patch.
> >> 
> >> Since -o is an XSI extension to POSIX, I've stuck with ||, but I think
> >> you are right that x would not be required in that case either.  
> > 
> > I'm not sure there are any common shells that don't support -o and -a.
> > They get used quite a lot.
> > I'm pretty sure they were supported by the pre-POSIX System-V shells
> > (or the /bin/[ program they ran).  
> 
> You are probably right.  I still remember when Debian policy allowed
> them and posh added support in 2007/2008:
> https://lists.debian.org/debian-devel/2006/11/msg00710.html
> (I was corrected by Clint Adams about -a and -o being XSI extensions
> some years before then when I noted posh lacked support, which is
> probably why I still remember it.)
> 
> I find && and || more readable, but I'm open to changing it if you
> feel strongly.

They get parsed entirely differently and are likely to be measurably slower.
Just FYI I tend not to use 'if' statements at all, just (eg):
	[ a = b ] && echo a == b

> Do I understand correctly that you are in favor of using the x prefix?
> I have a slight preference for leaving it off, but I'm open to adding
> it if you (or others) feel strongly.

I wouldn't take them out and consider shellcheck wrong, but the suffix
were just stupid.

	David

> Thanks for the interesting discussion,
> Kevin

Re: [PATCH] tools: remove unnecessary x suffix in test strings

[Kevin Locke]

On Sun, 2025-10-19 at 11:17 +0100, David Laight wrote:
> On Fri, 17 Oct 2025 16:28:12 -0600 Kevin Locke <kevin@kevinlocke.name> wrote:
>> On Fri, 2025-10-17 at 15:12 +0100, David Laight wrote:
>>> On Thu, 16 Oct 2025 17:47:09 -0600 Kevin Locke <kevin@kevinlocke.name> wrote:  
>>>> Remove the "x" suffixes which unnecessarily complicate the code.  
>>> 
>>> The problems arise when $1 is (say) "-x", a simple LR parser will treat
>>> [ -x = -x ] as a check for the file "=" being executable and then give
>>> a syntax error for the second -x.
>>> I can't imagine why shellcheck should warn about a leading x (or any other
>>> character) provided field splitting is disabled (eg by "").
>>> The leading x has definitely been needed in the past.  
>> 
>> Yep, it definitely has been.  The rationale on the wiki is that it's
>> not necessary for modern shells (and presumably that it unnecessarily
>> complicates the code): https://www.shellcheck.net/wiki/SC2268
>> However, it notes Zsh had issues as recently as 2015, which is not as
>> old as I would have expected.
> 
> It doesn't really make much difference to the shell.
> I really doubt you'll notice any difference in the time it takes to run.

I agree.  However, I'm more concerned about readability and
understandability for developers less familiar with the quirks of old
shells.

>>> POSIX does require the three argument 'test' look for the middle argument
>>> being an operator - but there might be historic shells that don't so that.
>>> OTOH you are probably looking for code from the early 1980s!
>>> But the POSIX spec (last time I read it) does point out the problems
>>> with arbitrary strings being treated as operators causing complex expressions
>>> be mis-parsed - which a leading x fixes.  
>> 
>> Good point.  I just reread it and can confirm that the current version
>> still notes issues mitigated by the X prefix with "historical shells"
>> and with greater than 4 argument cases:
>> https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html
> 
> The fact that the 'greater than 4 argument case' can still require
> a prefix character might be considered enough to make adding one all the
> time 'good practise' even though it (probably) isn't actually needed.

That seems reasonable to me, although I'd prefer omitting x and
prohibiting >3 argument cases, which appears to be the route
shellcheck takes with SC2268 + SC2166.

> While I wouldn't error not having a prefix, generating an error when
> there is one seems wrong.
> What does shellcheck do with [ "$a" = "$b" -o "$c" = "$d" ] ?

It only produces SC2166 (discouraging -o).  However, for 
[ "x$a" = "x$b" -o "x$c" = "x$d" ] it also produces SC2268.

> Or even [ "$a" "$b" "$c" "$d" "$e" "$f "$g" ] ??

This, and [ "$a" "$b" "$c" ] and [ "$a" "$b" ] produce parser error
SC1073.  Unfortunately, this appears to be a long-standing shellcheck
issue:  https://github.com/koalaman/shellcheck/issues/1645

>> I find && and || more readable, but I'm open to changing it if you
>> feel strongly.
> 
> They get parsed entirely differently and are likely to be measurably slower.
> Just FYI I tend not to use 'if' statements at all, just (eg):
> 	[ a = b ] && echo a == b
> 
>> Do I understand correctly that you are in favor of using the x prefix?
>> I have a slight preference for leaving it off, but I'm open to adding
>> it if you (or others) feel strongly.
> 
> I wouldn't take them out and consider shellcheck wrong, but the suffix
> were just stupid.

Are you opposed to the patch I posted removing the suffixes?  I had
tagged you as Suggested-by due to misreading your first post.  If the
change is not something you'd suggest, I can repost without it.

Thanks,
Kevin

Re: [PATCH] tools: remove unnecessary x suffix in test strings

[David Laight]

On Mon, 20 Oct 2025 14:18:32 -0600
Kevin Locke <kevin@kevinlocke.name> wrote:

> On Sun, 2025-10-19 at 11:17 +0100, David Laight wrote:
> > On Fri, 17 Oct 2025 16:28:12 -0600 Kevin Locke <kevin@kevinlocke.name> wrote:  
> >> On Fri, 2025-10-17 at 15:12 +0100, David Laight wrote:  
> >>> On Thu, 16 Oct 2025 17:47:09 -0600 Kevin Locke <kevin@kevinlocke.name> wrote:    
> >>>> Remove the "x" suffixes which unnecessarily complicate the code.    
> >>> 
> >>> The problems arise when $1 is (say) "-x", a simple LR parser will treat
> >>> [ -x = -x ] as a check for the file "=" being executable and then give
> >>> a syntax error for the second -x.
> >>> I can't imagine why shellcheck should warn about a leading x (or any other
> >>> character) provided field splitting is disabled (eg by "").
> >>> The leading x has definitely been needed in the past.    
> >> 
> >> Yep, it definitely has been.  The rationale on the wiki is that it's
> >> not necessary for modern shells (and presumably that it unnecessarily
> >> complicates the code): https://www.shellcheck.net/wiki/SC2268
> >> However, it notes Zsh had issues as recently as 2015, which is not as
> >> old as I would have expected.  
> > 
> > It doesn't really make much difference to the shell.
> > I really doubt you'll notice any difference in the time it takes to run.  
> 
> I agree.  However, I'm more concerned about readability and
> understandability for developers less familiar with the quirks of old
> shells.
> 
> >>> POSIX does require the three argument 'test' look for the middle argument
> >>> being an operator - but there might be historic shells that don't so that.
> >>> OTOH you are probably looking for code from the early 1980s!
> >>> But the POSIX spec (last time I read it) does point out the problems
> >>> with arbitrary strings being treated as operators causing complex expressions
> >>> be mis-parsed - which a leading x fixes.    
> >> 
> >> Good point.  I just reread it and can confirm that the current version
> >> still notes issues mitigated by the X prefix with "historical shells"
> >> and with greater than 4 argument cases:
> >> https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html  
> > 
> > The fact that the 'greater than 4 argument case' can still require
> > a prefix character might be considered enough to make adding one all the
> > time 'good practise' even though it (probably) isn't actually needed.  
> 
> That seems reasonable to me, although I'd prefer omitting x and
> prohibiting >3 argument cases, which appears to be the route
> shellcheck takes with SC2268 + SC2166.

Ugg.
I know the parser is 'problematic' but you need -o (and -a) to get
moderately efficient expression evaluation.
If shellcheck objects to those I'd guess it also objects to ( and ).

You really don't want to use [ ... ] && [ ... ] because it goes right
out to the command pipeline parser.
Not to mention the lack of grouping for a || b && c


> > While I wouldn't error not having a prefix, generating an error when
> > there is one seems wrong.
> > What does shellcheck do with [ "$a" = "$b" -o "$c" = "$d" ] ?  
> 
> It only produces SC2166 (discouraging -o).  However, for 
> [ "x$a" = "x$b" -o "x$c" = "x$d" ] it also produces SC2268.
> 
> > Or even [ "$a" "$b" "$c" "$d" "$e" "$f "$g" ] ??  
> 
> This, and [ "$a" "$b" "$c" ] and [ "$a" "$b" ] produce parser error
> SC1073.  Unfortunately, this appears to be a long-standing shellcheck
> issue:  https://github.com/koalaman/shellcheck/issues/1645
> 
> >> I find && and || more readable, but I'm open to changing it if you
> >> feel strongly.  
> > 
> > They get parsed entirely differently and are likely to be measurably slower.
> > Just FYI I tend not to use 'if' statements at all, just (eg):
> > 	[ a = b ] && echo a == b
> >   
> >> Do I understand correctly that you are in favor of using the x prefix?
> >> I have a slight preference for leaving it off, but I'm open to adding
> >> it if you (or others) feel strongly.  
> > 
> > I wouldn't take them out and consider shellcheck wrong, but the suffix
> > were just stupid.  
> 
> Are you opposed to the patch I posted removing the suffixes?  I had
> tagged you as Suggested-by due to misreading your first post.  If the
> change is not something you'd suggest, I can repost without it.

The suffixes are just wrong.
If the shell treats the first parameter to [ as an operator and $1 is "-"
it processes [ -x = ... and looks for a file "=".
Without the suffix the same happens when $1 is "-x".

A conformant shell won't do this for a 3-argument [.
But I also suspect that any conformant shell supports -o and -a.
The 7-argument [ definitely needs the prefix to protect against unexpected
operators.

So I still think shellcheck is just wrong here.
It ought to be checking FOR a prefix when there are 4 or more arguments.
It is one of those idioms you have to get used to.

But at the end of the day it is probably your call.

	David

> 
> Thanks,
> Kevin