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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251019111748.3d5ac8d9@pumpkin>
Date: Sun, 19 Oct 2025 11:17:48 +0100
From: David Laight <david.laight.linux@...il.com>
To: Kevin Locke <kevin@...inlocke.name>
Cc: Jonathan Corbet <corbet@....net>, Randy Dunlap <rdunlap@...radead.org>,
 Thorsten Leemhuis <linux@...mhuis.info>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tools: remove unnecessary x suffix in test strings

On Fri, 17 Oct 2025 16:28:12 -0600
Kevin Locke <kevin@...inlocke.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@...inlocke.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