Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src PR bin/54112



details:   https://anonhg.NetBSD.org/src/rev/1add0a09061d
branches:  trunk
changeset: 450316:1add0a09061d
user:      kre <kre%NetBSD.org@localhost>
date:      Wed Apr 10 08:13:11 2019 +0000

description:
PR bin/54112

Fix handling of "$@" (that is, double quoted dollar at), when it
appears in a string which will be subject to field splitting.

Eg:
        ${0+"$@" }

More common usages, like the simple "$@" or ${0+"$@"} end up
being entirely quoted, so no field splitting happens, and the
problem was avoided.

See the PR for more details.

This ends up making a bunch of old hack code (and some that was
relatively new) vanish - for now it is just #if 0'd or commented out.
Cleanups of that stuff will happen later.

That some of the worst $@ hacks are now gone does not mean that processing
of "$@" does not retain a very special place in every hackers heart.
RIP extreme ugliness - long live the merely ordinary ugly.

Added a new bin/sh ATF test case to verify that all this remains fixed.

diffstat:

 bin/sh/expand.c          |   43 +++++++++++++++++--
 tests/bin/sh/t_expand.sh |  105 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 119 insertions(+), 29 deletions(-)

diffs (truncated from 425 to 300 lines):

diff -r 1ccd55e9d88a -r 1add0a09061d bin/sh/expand.c
--- a/bin/sh/expand.c   Wed Apr 10 06:58:12 2019 +0000
+++ b/bin/sh/expand.c   Wed Apr 10 08:13:11 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: expand.c,v 1.131 2019/02/27 04:10:56 kre Exp $ */
+/*     $NetBSD: expand.c,v 1.132 2019/04/10 08:13:11 kre Exp $ */
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c   8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.131 2019/02/27 04:10:56 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.132 2019/04/10 08:13:11 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -97,6 +97,8 @@
 struct ifsregion *ifslastp;    /* last struct in list */
 struct arglist exparg;         /* holds expanded arg list */
 
+static int empty_dollar_at;    /* have expanded "$@" to nothing */
+
 STATIC const char *argstr(const char *, int);
 STATIC const char *exptilde(const char *, int);
 STATIC void expbackq(union node *, int, int);
@@ -180,6 +182,7 @@
        if (fflag)              /* no filename expandsion */
                flag &= ~EXP_GLOB;
 
+       empty_dollar_at = 0;
        argbackq = arg->narg.backquote;
        STARTSTACKSTR(expdest);
        ifsfirst.next = NULL;
@@ -243,6 +246,8 @@
        char c;
        const int quotes = flag & EXP_QNEEDED;          /* do CTLESC */
        int firsteq = 1;
+       int had_dol_at = 0;
+       int startoff;
        const char *ifs = NULL;
        int ifs_split = EXP_IFS_SPLIT;
 
@@ -251,6 +256,7 @@
 
        CTRACE(DBG_EXPAND, ("argstr(\"%s\", %#x) quotes=%#x\n", p,flag,quotes));
 
+       startoff = expdest - stackblock();
        if (*p == '~' && (flag & (EXP_TILDE | EXP_VARTILDE)))
                p = exptilde(p, flag);
        for (;;) {
@@ -262,6 +268,8 @@
                        return p - 1;
                case CTLENDVAR: /* end of expanding yyy in ${xxx-yyy} */
                case CTLENDARI: /* end of a $(( )) string */
+                       if (had_dol_at && (*p&0xFF) == CTLQUOTEEND)
+                               p++;
                        NULLTERM_4_TRACE(expdest);
                        VTRACE(DBG_EXPAND, ("argstr returning at \"%.6s\"..."
                            " after %2.2X; added \"%s\" to expdest\n",
@@ -270,8 +278,12 @@
                case CTLQUOTEMARK:
                        /* "$@" syntax adherence hack */
                        if (p[0] == CTLVAR && p[1] & VSQUOTE &&
-                           p[2] == '@' && p[3] == '=')
+                           p[2] == '@' && p[3] == '=') {
+                               had_dol_at = 1;
                                break;
+                       }
+                       had_dol_at = 0;
+                       empty_dollar_at = 0;
                        if ((flag & EXP_SPLIT) != 0)
                                STPUTC(c, expdest);
                        ifs_split = 0;
@@ -285,9 +297,14 @@
                        STPUTC('\n', expdest);  /* no line_number++ */
                        break;
                case CTLQUOTEEND:
-                       if ((flag & EXP_SPLIT) != 0)
+                       if (empty_dollar_at &&
+                           expdest - stackblock() > startoff &&
+                           expdest[-1] == CTLQUOTEMARK)
+                               expdest--;
+                       else if (!had_dol_at && (flag & EXP_SPLIT) != 0)
                                STPUTC(c, expdest);
                        ifs_split = EXP_IFS_SPLIT;
+                       had_dol_at = 0;
                        break;
                case CTLESC:
                        if (quotes || ISCTL(*p))
@@ -890,6 +907,8 @@
        } else if (special) {
                set = varisset(var, varflags & VSNUL);
                val = NULL;
+               if (!set && *var == '@')
+                       empty_dollar_at = 1;
        } else {
                val = lookupvar(var);
                if (val == NULL || ((varflags & VSNUL) && val[0] == '\0')) {
@@ -916,9 +935,11 @@
                }
        }
 
+#if 0          /* no longer need this $@ evil ... */
        if (!set && subtype != VSPLUS && special && *var == '@')
                if (startloc > 0 && expdest[-1] == CTLQUOTEMARK)
                        expdest--, startloc--;
+#endif
 
        if (set && subtype != VSPLUS) {
                /* insert the value of the variable */
@@ -1202,13 +1223,23 @@
                if (flag & EXP_SPLIT && quoted) {
                        VTRACE(DBG_EXPAND, (": $@ split (%d)\n",
                            shellparam.nparam));
+#if 0
                /* GROSS HACK */
                        if (shellparam.nparam == 0 &&
                                expdest[-1] == CTLQUOTEMARK)
                                        expdest--;
                /* KCAH SSORG */
+#endif
+                       if (shellparam.nparam == 0)
+                               empty_dollar_at = 1;
+
                        for (ap = shellparam.p ; (p = *ap++) != NULL ; ) {
-                               STRTODEST(p);
+                               if (*p == '\0') {
+                                       /* retain an explicit null string */
+                                       STPUTC(CTLQUOTEMARK, expdest);
+                                       STPUTC(CTLQUOTEEND, expdest);
+                               } else
+                                       STRTODEST(p);
                                if (*ap)
                                        /* A NUL separates args inside "" */
                                        STPUTC('\0', expdest);
@@ -1396,8 +1427,10 @@
                }
        }
 
+/*
        while (*start == CTLQUOTEEND)
                start++;
+*/
 
        /*
         * Save anything left as an argument.
diff -r 1ccd55e9d88a -r 1add0a09061d tests/bin/sh/t_expand.sh
--- a/tests/bin/sh/t_expand.sh  Wed Apr 10 06:58:12 2019 +0000
+++ b/tests/bin/sh/t_expand.sh  Wed Apr 10 08:13:11 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: t_expand.sh,v 1.20 2018/11/27 09:59:30 kre Exp $
+# $NetBSD: t_expand.sh,v 1.21 2019/04/10 08:13:11 kre Exp $
 #
 # Copyright (c) 2007, 2009 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -46,7 +46,7 @@
 
 atf_test_case dollar_at
 dollar_at_head() {
-       atf_set "descr" "Somewhere between 2.0.2 and 3.0 the expansion" \
+       atf_set descr "Somewhere between 2.0.2 and 3.0 the expansion" \
                        "of the \$@ variable had been broken.  Check for" \
                        "this behavior."
 }
@@ -65,7 +65,7 @@
 
 atf_test_case dollar_at_unquoted_or_conditional
 dollar_at_unquoted_or_conditional_head() {
-       atf_set "descr" 'Sometime during 2013 the expansion of "${1+$@}"' \
+       atf_set descr 'Sometime during 2013 the expansion of "${1+$@}"' \
                        ' (where $1 and $2 (and maybe more) are set)' \
                        ' seems to have broken.  Check for this bug.'
 }
@@ -87,7 +87,7 @@
 
 atf_test_case dollar_at_with_text
 dollar_at_with_text_head() {
-       atf_set "descr" "Test \$@ expansion when it is surrounded by text" \
+       atf_set descr "Test \$@ expansion when it is surrounded by text" \
                        "within the quotes.  PR bin/33956."
 }
 dollar_at_with_text_body() {
@@ -160,7 +160,7 @@
 
 atf_test_case dollar_at_empty_and_conditional
 dollar_at_empty_and_conditional_head() {
-       atf_set "descr" 'Test $@ expansion when there are no args, and ' \
+       atf_set descr 'Test $@ expansion when there are no args, and ' \
                        'when conditionally expanded.'
 }
 dollar_at_empty_and_conditional_body() {
@@ -355,7 +355,7 @@
 
 atf_test_case strip
 strip_head() {
-       atf_set "descr" "Checks that the %% operator works and strips" \
+       atf_set descr "Checks that the %% operator works and strips" \
                        "the contents of a variable from the given point" \
                        "to the end"
 }
@@ -385,7 +385,7 @@
 
 atf_test_case wrap_strip
 wrap_strip_head() {
-       atf_set "descr" "Checks that the %% operator works and strips" \
+       atf_set descr "Checks that the %% operator works and strips" \
                        "the contents of a variable from the given point" \
                        'to the end, and that \ \n sequences do not break it'
 }
@@ -429,7 +429,7 @@
 
 atf_test_case tilde
 tilde_head() {
-       atf_set "descr" "Checks that the ~ expansions work"
+       atf_set descr "Checks that the ~ expansions work"
 }
 tilde_body() {
        for HOME in '' / /home/foo \
@@ -489,7 +489,7 @@
 
 atf_test_case varpattern_backslashes
 varpattern_backslashes_head() {
-       atf_set "descr" "Tests that protecting wildcards with backslashes" \
+       atf_set descr "Tests that protecting wildcards with backslashes" \
                        "works in variable patterns."
 }
 varpattern_backslashes_body() {
@@ -501,7 +501,7 @@
 
 atf_test_case arithmetic
 arithmetic_head() {
-       atf_set "descr" "POSIX requires shell arithmetic to use signed" \
+       atf_set descr "POSIX requires shell arithmetic to use signed" \
                        "long or a wider type.  We use intmax_t, so at" \
                        "least 64 bits should be available.  Make sure" \
                        "this is true."
@@ -518,7 +518,7 @@
 
 atf_test_case iteration_on_null_parameter
 iteration_on_null_parameter_head() {
-       atf_set "descr" "Check iteration of \$@ in for loop when set to null;" \
+       atf_set descr "Check iteration of \$@ in for loop when set to null;" \
                        "the error \"sh: @: parameter not set\" is incorrect." \
                        "PR bin/48202."
 }
@@ -529,7 +529,7 @@
 
 atf_test_case iteration_on_quoted_null_parameter
 iteration_on_quoted_null_parameter_head() {
-       atf_set "descr" \
+       atf_set descr \
                'Check iteration of "$@" in for loop when set to null;'
 }
 iteration_on_quoted_null_parameter_body() {
@@ -539,7 +539,7 @@
 
 atf_test_case iteration_on_null_or_null_parameter
 iteration_on_null_or_null_parameter_head() {
-       atf_set "descr" \
+       atf_set descr \
                'Check expansion of null parameter as default for another null'
 }
 iteration_on_null_or_null_parameter_body() {
@@ -549,7 +549,7 @@
 
 atf_test_case iteration_on_null_or_missing_parameter
 iteration_on_null_or_missing_parameter_head() {
-       atf_set "descr" \
+       atf_set descr \
            'Check expansion of missing parameter as default for another null'
 }
 iteration_on_null_or_missing_parameter_body() {
@@ -666,7 +666,7 @@
 
 atf_test_case shell_params
 shell_params_head() {
-       atf_set "descr" "Test correct operation of the numeric parameters"
+       atf_set descr "Test correct operation of the numeric parameters"
 }
 shell_params_body() {
        atf_require_prog mktemp
@@ -725,7 +725,7 @@
 
 atf_test_case var_with_embedded_cmdsub
 var_with_embedded_cmdsub_head() {
-       atf_set "descr" "Test expansion of vars with embedded cmdsub"
+       atf_set descr "Test expansion of vars with embedded cmdsub"
 }
 var_with_embedded_cmdsub_body() {
 
@@ -782,7 +782,7 @@
 
 atf_test_case dollar_hash
 dollar_hash_head() {
-       atf_set "descr" 'Test expansion of various aspects of $#'
+       atf_set descr 'Test expansion of various aspects of $#'
 }
 dollar_hash_body() {



Home | Main Index | Thread Index | Old Index