From: Peter Stephenson <p.stephenson@samsung.com>
To: zsh-workers@zsh.org
Subject: Re: Bug: [ "(" = ")" ] is true
Date: Wed, 09 Dec 2015 16:24:28 +0000 [thread overview]
Message-ID: <20151209162428.1ad53b89@pwslap01u.europe.root.pri> (raw)
In-Reply-To: <56684A0A.8010404@inlv.org>
On Wed, 9 Dec 2015 16:34:34 +0100
Martijn Dekker <martijn@inlv.org> wrote:
> There is a string comparison bug with `[' and `test'; the result is true
> if the first string starts with '(' and the second string starts with ')'.
>
> $ [ "(" = ")" ] && echo oops || echo ok
> oops
This is yet another result of a folorn attempt to handle extensions of
the "[" syntax consistently. We can't tell if "(" ... ")" are for group
or are arguments to a command, so we guess. However, as groups are an
extension of the most basic syntax, we should first try the latter.
Note that
[ "(" string ")" ]
and
[ "(" "" ")" ]
are valid tests that return true and false respectively. (They tend to
suggest that whoever wrote those tests has lost the plot, but, well.)
> $ [ "((" = "))" ] && echo oops || echo ok
> oops
This seems to be a more obvious bug that we only care about the first
character. I can't see how that could ever possibly be right. It's
possible there may have been some confusion over the fact that if we
were parsing this properly for "[[" then the "(" and ")" wouldn't
need to be in separate words, but that doesn't apply to "test" / "[".
Added some "const"s in text.c with a little trepidation --- but I think
it's a long time since we had problems with older OSes in that department.
diff --git a/Src/builtin.c b/Src/builtin.c
index cac4f42..b06bc6d 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -6463,7 +6463,13 @@ bin_test(char *name, char **argv, UNUSED(Options ops), int func)
nargs = arrlen(argv);
if (nargs == 3 || nargs == 4)
{
- if (*argv[0] == '(' && *argv[nargs-1] == ')') {
+ /*
+ * As parentheses are an extension, we need to be careful ---
+ * if this is a three-argument expression that could
+ * be a binary operator, prefer that.
+ */
+ if (!strcmp(argv[0], "(") && !strcmp(argv[nargs-1],")") &&
+ (nargs != 3 || !is_cond_binary_op(argv[1]))) {
argv[nargs-1] = NULL;
argv++;
}
diff --git a/Src/text.c b/Src/text.c
index 9421d70..04acd2a 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -40,9 +40,32 @@
/**/
int text_expand_tabs;
+/*
+ * Binary operators in conditions.
+ * There order is tied to the order of the definitions COND_STREQ
+ * et seq. in zsh.h.
+ */
+static const char *cond_binary_ops[] = {
+ "=", "!=", "<", ">", "-nt", "-ot", "-ef", "-eq",
+ "-ne", "-lt", "-gt", "-le", "-ge", "=~"
+};
+
static char *tptr, *tbuf, *tlim, *tpending;
static int tsiz, tindent, tnewlins, tjob;
+/**/
+int
+is_cond_binary_op(const char *str)
+{
+ const char **op;
+ for (op = cond_binary_ops; *op; op++)
+ {
+ if (!strcmp(str, *op))
+ return 1;
+ }
+ return 0;
+}
+
static void
dec_tindent(void)
{
@@ -120,7 +143,7 @@ taddchr(int c)
/**/
static void
-taddstr(char *s)
+taddstr(const char *s)
{
int sl = strlen(s);
char c;
@@ -822,11 +845,6 @@ gettext2(Estate state)
break;
case WC_COND:
{
- static char *c1[] = {
- "=", "!=", "<", ">", "-nt", "-ot", "-ef", "-eq",
- "-ne", "-lt", "-gt", "-le", "-ge", "=~"
- };
-
int ctype;
if (!s) {
@@ -912,7 +930,7 @@ gettext2(Estate state)
/* Binary test: `a = b' etc. */
taddstr(ecgetstr(state, EC_NODUP, NULL));
taddstr(" ");
- taddstr(c1[ctype - COND_STREQ]);
+ taddstr(cond_binary_ops[ctype - COND_STREQ]);
taddstr(" ");
taddstr(ecgetstr(state, EC_NODUP, NULL));
if (ctype == COND_STREQ ||
diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 40bbf42..9e13696 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -389,6 +389,18 @@ F:Failures in these cases do not indicate a problem in the shell.
>Not zero 5
>Not zero 6
+ [ '(' = ')' ] || print OK 1
+ [ '((' = '))' ] || print OK 2
+ [ '(' = '(' ] && print OK 3
+ [ '(' non-empty-string ')' ] && echo OK 4
+ [ '(' '' ')' ] || echo OK 5
+0:yet more old-fashioned test fix ups: prefer comparison to parentheses
+>OK 1
+>OK 2
+>OK 3
+>OK 4
+>OK 5
+
%clean
# This works around a bug in rm -f in some versions of Cygwin
chmod 644 unmodish
prev parent reply other threads:[~2015-12-09 16:24 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-09 15:34 Martijn Dekker
2015-12-09 15:58 ` Stephane Chazelas
2015-12-09 16:31 ` Martijn Dekker
2015-12-09 20:27 ` Bart Schaefer
2015-12-09 16:24 ` Peter Stephenson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151209162428.1ad53b89@pwslap01u.europe.root.pri \
--to=p.stephenson@samsung.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).