Pádraig Brady <[email protected]> writes:
> On 25/07/2026 18:54, Collin Funk wrote:
>> The attached patch should fix the issue. We were treating "-o" and "-a"
>> as if they could be unary operators, which they cannot be. They are
>> either a string operand or a binary operator.
>> Will push it in a bit.
>
> Very nice. This now matches behavior of bash and dash at least.
> One nit on the NEWS, I was a bit confused at the double mention
> of binary operators, when perhaps unary was more accurate.
> Therefore it might be best to simplify the first line to:
>
> 'test' no longer treats '-a' and '-o' as operators when given as
Right. I was trying to make the entry friendly to those who may not be
familiar with the terminology, hence the added example. While thinking
of how to word it I must have made that silly mistake.
I pushed this v2 patch with the changes in two_arguments removed. It
caused an incorrect error message, as noticed by
tests/test/test-diag.pl:
$ ./src/test -o arg
test: missing argument after ‘arg’
Whereas, in v2 it gives the more correct error, telling us that -o is
not a unary operator:
$ ./src/test -o arg
test: ‘-o’: unary operator expected
I'm not too sure why I added it in v1, to be honest. I added a check for
-a in tests/test/test-diag.pl, just because it seems nice to cover that
case as well.
One thing I found confusing is that 'bash' doesn't have error messages
in the following cases:
$ test -o -o; echo $?
1
$ test -a -a; echo $?
1
Before testing both, I tested 'test -a -a' and thought it might have
been treated as the same as:
$ test -a -a ''
But we can see that isn't the case from this:
$ test -o -o; echo $?
1
$ test -o -o ''; echo $?
0
I probably put too much thought into these seldom used operators. :)
Collin
>From c2a2508c380402865124d6101856f489e6dd04f8 Mon Sep 17 00:00:00 2001
Message-ID: <c2a2508c380402865124d6101856f489e6dd04f8.1785096019.git.collin.fu...@gmail.com>
From: Collin Funk <[email protected]>
Date: Thu, 23 Jul 2026 21:12:42 -0700
Subject: [PATCH v2] test: handle adjacent binary operators
* NEWS: Mention the bug fix.
* cfg.mk (exclude_file_name_regexp--sc_prohibit_test_minus_ao): Exclude
NEWS.
* src/test.c (term): Treat '-a' and '-o' as strings instead of expecting
them to be valid unary operators.
* tests/test/test.pl: Add some test cases.
* tests/test/test-diag.pl: Also test the diagnostics with '-a arg'.
---
NEWS | 5 +++++
cfg.mk | 2 +-
src/test.c | 3 ++-
tests/test/test-diag.pl | 5 ++---
tests/test/test.pl | 18 ++++++++++++++++++
5 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/NEWS b/NEWS
index d47d9517e..fd0902cb7 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,11 @@ GNU coreutils NEWS -*- outline -*-
'tee' no longer treats short writes as errors.
[bug introduced in coreutils-9.11]
+ 'test' no longer treats '-a' and '-o' as operators when given as strings to a
+ binary operator. E.g., 'test -a -a -a' exits successfully instead of exiting
+ with an error.
+ [This bug was present in "the beginning".]
+
'unexpand -t' no longer overflows a heap buffer, for tab values > SIZE_MAX/16,
or with multi-byte blank characters longer than the tab value.
[bugs introduced in coreutils-9.11]
diff --git a/cfg.mk b/cfg.mk
index 90e6a21d1..b21b7dc03 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -950,7 +950,7 @@ exclude_file_name_regexp--sc_prohibit_always_true_header_tests = \
^m4/stat-prog\.m4$$
exclude_file_name_regexp--sc_prohibit_fail_0 = \
(^.*/git-hooks/commit-msg|^tests/init\.sh|Makefile\.am|\.mk|.*\.texi)$$
-exclude_file_name_regexp--sc_prohibit_test_minus_ao = doc/.*\.texi$$
+exclude_file_name_regexp--sc_prohibit_test_minus_ao = NEWS|doc/.*\.texi$$
exclude_file_name_regexp--sc_prohibit_atoi_atof = ^lib/euidaccess-stat\.c$$
# longlong.h is maintained elsewhere.
diff --git a/src/test.c b/src/test.c
index 692c37263..f28fb7c9d 100644
--- a/src/test.c
+++ b/src/test.c
@@ -265,7 +265,8 @@ term (void)
value = binary_operator (false, bop);
/* It might be a switch type argument. */
- else if (argv[pos][0] == '-' && argv[pos][1] && argv[pos][2] == '\0')
+ else if (argv[pos][0] == '-' && argv[pos][1] && argv[pos][1] != 'a'
+ && argv[pos][1] != 'o' && argv[pos][2] == '\0')
value = unary_operator ();
else
{
diff --git a/tests/test/test-diag.pl b/tests/test/test-diag.pl
index b9929d9c5..8a2557045 100755
--- a/tests/test/test-diag.pl
+++ b/tests/test/test-diag.pl
@@ -26,9 +26,8 @@ use strict;
my @Tests =
(
# In coreutils-5.93, this diagnostic lacked the newline.
- ['o', '-o arg', {ERR => "test: '-o': unary operator expected\n"},
- {ERR_SUBST => 's!^.*test:!test:!'},
- {EXIT => 2}],
+ map {[$_, "-$_ arg", {ERR => "test: '-$_': unary operator expected\n"},
+ {ERR_SUBST => 's!^.*test:!test:!'}, {EXIT => 2}]} qw(o a),
);
my $save_temps = $ENV{DEBUG};
diff --git a/tests/test/test.pl b/tests/test/test.pl
index 0f0a93d67..9b722191d 100755
--- a/tests/test/test.pl
+++ b/tests/test/test.pl
@@ -204,6 +204,24 @@ my @Tests =
['greater-collate-3', "'a' '>' 'b'", {EXIT=>1}],
);
+# Test the behavior of multiple adjacent binary operators.
+# These would not behave correctly in coreutils-9.11 and earlier.
+my $i = 1;
+my @operands = ('-a', '-o', "''");
+for my $b ('-a', '-o')
+ {
+ for my $a (@operands)
+ {
+ for my $c (@operands)
+ {
+ push @Tests, ["multiple-binary-" . ++$i, "$a $b $c",
+ {EXIT=>0 + (($b eq '-o')
+ ? ($a eq "''" && $c eq "''")
+ : ($a eq "''" || $c eq "''"))}];
+ }
+ }
+ }
+
@Tests = add_inverse_op_tests \@Tests;
@Tests = add_pn_tests \@Tests;
--
2.55.0
