ash: expand: Fix ghost fields with unquoted $@/$*

Upstream commit:

    Date: Fri, 23 Mar 2018 18:58:47 +0800
    expand: Fix ghost fields with unquoted $@/$*

    You're right.  The proper fix to this is to ensure that nulonly
    is not set in varvalue for $*.  It should only be set for $@ when
    it's inside double quotes.

    In fact there is another bug while we're playing with $@/$*.
    When IFS is set to a non-whitespace character such as :, $*
    outside quotes won't remove empty fields as it should.

    This patch fixes both problems.

    Reported-by: Martijn Dekker <martijn@inlv.org>
    Suggested-by: Harald van Dijk <harald@gigawatt.nl>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function                                             old     new   delta
argstr                                              1111    1113      +2
evalvar                                              571     569      -2
varvalue                                             579     576      -3
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/2 up/down: 2/-5)               Total: -3 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/shell/ash.c b/shell/ash.c
index 6ef0a7ac272977c3746d080a2c5fa8f4164fa000..4641dfd194bebe16585fd63e9855fd2e2e10e3ee 100644 (file)
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -5902,7 +5902,7 @@ static int substr_atoi(const char *s)
 #define EXP_CASE        0x10    /* keeps quotes around for CASE pattern */
 #define EXP_VARTILDE2   0x20    /* expand tildes after colons only */
 #define EXP_WORD        0x40    /* expand word in parameter expansion */
-#define EXP_QUOTED      0x80    /* expand word in double quotes */
+#define EXP_QUOTED      0x100   /* expand word in double quotes */
 /*
  * rmescape() flags
  */
@@ -7175,14 +7175,13 @@ subevalvar(char *p, char *varname, int strloc, int subtype,
  * ash -c 'echo ${#1#}'  name:'1=#'
  */
 static NOINLINE ssize_t
-varvalue(char *name, int varflags, int flags, int *quotedp)
+varvalue(char *name, int varflags, int flags, int quoted)
 {
        const char *p;
        int num;
        int i;
        ssize_t len = 0;
        int sep;
-       int quoted = *quotedp;
        int subtype = varflags & VSTYPE;
        int discard = subtype == VSPLUS || subtype == VSLENGTH;
        int quotes = (discard ? 0 : (flags & QUOTES_ESC)) | QUOTES_KEEPNUL;
@@ -7230,13 +7229,27 @@ varvalue(char *name, int varflags, int flags, int *quotedp)
        case '*': {
                char **ap;
                char sepc;
+               char c;
 
-               if (quoted)
-                       sep = 0;
-               sep |= ifsset() ? ifsval()[0] : ' ';
+               /* We will set c to 0 or ~0 depending on whether
+                * we're doing field splitting.  We won't do field
+                * splitting if either we're quoted or sep is zero.
+                *
+                * Instead of testing (quoted || !sep) the following
+                * trick optimises away any branches by using the
+                * fact that EXP_QUOTED (which is the only bit that
+                * can be set in quoted) is the same as EXP_FULL <<
+                * CHAR_BIT (which is the only bit that can be set
+                * in sep).
+                */
+#if EXP_QUOTED >> CHAR_BIT != EXP_FULL
+#error The following two lines expect EXP_QUOTED == EXP_FULL << CHAR_BIT
+#endif
+               c = !((quoted | ~sep) & EXP_QUOTED) - 1;
+               sep &= ~quoted;
+               sep |= ifsset() ? (unsigned char)(c & ifsval()[0]) : ' ';
  param:
                sepc = sep;
-               *quotedp = !sepc;
                ap = shellparam.p;
                if (!ap)
                        return -1;
@@ -7301,7 +7314,6 @@ evalvar(char *p, int flag)
        char varflags;
        char subtype;
        int quoted;
-       char easy;
        char *var;
        int patloc;
        int startloc;
@@ -7315,12 +7327,11 @@ evalvar(char *p, int flag)
 
        quoted = flag & EXP_QUOTED;
        var = p;
-       easy = (!quoted || (*var == '@' && shellparam.nparam));
        startloc = expdest - (char *)stackblock();
        p = strchr(p, '=') + 1; //TODO: use var_end(p)?
 
  again:
-       varlen = varvalue(var, varflags, flag, &quoted);
+       varlen = varvalue(var, varflags, flag, quoted);
        if (varflags & VSNUL)
                varlen--;
 
@@ -7366,8 +7377,11 @@ evalvar(char *p, int flag)
 
        if (subtype == VSNORMAL) {
  record:
-               if (!easy)
-                       goto end;
+               if (quoted) {
+                       quoted = *var == '@' && shellparam.nparam;
+                       if (!quoted)
+                               goto end;
+               }
                recordregion(startloc, expdest - (char *)stackblock(), quoted);
                goto end;
        }

diff --git a/shell/ash_test/ash-vars/var_wordsplit_ifs5.right b/shell/ash_test/ash-vars/var_wordsplit_ifs5.right
new file mode 100644 (file)
index 0000000..46ffcec
--- /dev/null
+++ b/shell/ash_test/ash-vars/var_wordsplit_ifs5.right
@@ -0,0 +1 @@
+Zero:0

diff --git a/shell/ash_test/ash-vars/var_wordsplit_ifs5.tests b/shell/ash_test/ash-vars/var_wordsplit_ifs5.tests
new file mode 100755 (executable)
index 0000000..d382116
--- /dev/null
+++ b/shell/ash_test/ash-vars/var_wordsplit_ifs5.tests
@@ -0,0 +1,4 @@
+IFS=
+set --
+set -- $@ $*
+echo Zero:$#

diff --git a/shell/hush_test/hush-vars/var_wordsplit_ifs5.right b/shell/hush_test/hush-vars/var_wordsplit_ifs5.right
new file mode 100644 (file)
index 0000000..46ffcec
--- /dev/null
+++ b/shell/hush_test/hush-vars/var_wordsplit_ifs5.right
@@ -0,0 +1 @@
+Zero:0

diff --git a/shell/hush_test/hush-vars/var_wordsplit_ifs5.tests b/shell/hush_test/hush-vars/var_wordsplit_ifs5.tests
new file mode 100755 (executable)
index 0000000..d382116
--- /dev/null
+++ b/shell/hush_test/hush-vars/var_wordsplit_ifs5.tests
@@ -0,0 +1,4 @@
+IFS=
+set --
+set -- $@ $*
+echo Zero:$#