ash: eval: Variable assignments on functions are no longer persistent

Upstream commit:

    Date: Wed, 4 Apr 2018 17:54:01 +0800
    eval: Variable assignments on functions are no longer persistent

    Dirk Fieldhouse <fieldhouse@gmx.net> wrote:
    > In POSIX.1-2017 ("simultaneously IEEE Std 1003.1™-2017 and The Open
    > Group Technical Standard Base Specifications, Issue 7")
    > <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09>,
    > we read under '2.9.1 Simple Commands'
    >
    > "Variable assignments shall be performed as follows:
    > ...
    > -    If the command name is a standard utility implemented as a function
    > (see XBD Utility), the effect of variable assignments shall be as if the
    > utility was not implemented as a function.
    > ...
    > -    If the command name is a function that is not a standard utility
    > implemented as a function, variable assignments shall affect the current
    > execution environment during the execution of the function. It is
    > unspecified:
    >
    >     *   Whether or not the variable assignments persist after the
    > completion of the function
    >
    >     *   Whether or not the variables gain the export attribute during
    > the execution of the function
    >
    >     *   Whether or not export attributes gained as a result of the
    > variable assignments persist after the completion of the function (if
    > variable assignments persist after the completion of the function)"

    POSIX used to require the current dash behaviour.  However, you're
    right that this is no longer the case.

    This patch will remove the persistence of the variable assignment.

    I have considered the exporting the variables during the function
    execution but have decided against it because:

    1) It makes the code bigger.
    2) dash has never done this in the past.
    3) You cannot use this portably anyway.

    Reported-by: Dirk Fieldhouse <fieldhouse@gmx.net>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function                                             old     new   delta
evalcommand                                         1606    1635     +29
evalcase                                             313     317      +4
evalfun                                              280     268     -12
pushlocalvars                                         48       -     -48
------------------------------------------------------------------------------
(add/remove: 0/1 grow/shrink: 2/1 up/down: 33/-60)            Total: -27 bytes

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

diff --git a/shell/ash.c b/shell/ash.c
index 5c431c9ffb5fa2c079f502bd46e61bd2f8b63649..6cda7251e7affb726387cfa4fa8021f73adfda08 100644 (file)
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -9600,9 +9600,7 @@ evalfun(struct funcnode *func, int argc, char **argv, int flags)
        shellparam.optind = 1;
        shellparam.optoff = -1;
 #endif
-       pushlocalvars();
        evaltree(func->n.ndefun.body, flags & EV_TESTED);
-       poplocalvars(0);
  funcdone:
        INT_OFF;
        funcline = savefuncline;
@@ -10235,7 +10233,6 @@ evalcommand(union node *cmd, int flags)
                goto readstatus;
 
        case CMDFUNCTION:
-               poplocalvars(1);
                /* See above for the rationale */
                dowait(DOWAIT_NONBLOCK, NULL);
                if (evalfun(cmdentry.u.func, argc, argv, flags))

diff --git a/shell/ash_test/ash-misc/env_and_func.right b/shell/ash_test/ash-misc/env_and_func.right
index 5fc3488aef94d6abf2a89b9d90804a2e2737c1a3..4a15450586817df05c5439ca50cd70c92dfce0e6 100644 (file)
--- a/shell/ash_test/ash-misc/env_and_func.right
+++ b/shell/ash_test/ash-misc/env_and_func.right
@@ -1,2 +1,2 @@
 var=val
-var=val
+var=old

diff --git a/shell/ash_test/ash-misc/env_and_func.tests b/shell/ash_test/ash-misc/env_and_func.tests
index 3efef1a41945e92f7f3f96033a793d5f9831ed1b..1c63eafd80270d1d39073273db336b2ccf96bcfd 100755 (executable)
--- a/shell/ash_test/ash-misc/env_and_func.tests
+++ b/shell/ash_test/ash-misc/env_and_func.tests
@@ -3,6 +3,6 @@ f() { echo "var=$var"; }
 # bash: POSIXLY_CORRECT behavior is to "leak" new variable values
 # out of function invocations (similar to "special builtins" behavior);
 # but in "bash mode", they don't leak.
-# hush does not "leak" values. ash does.
+# hush does not "leak" values. ash used to, but now does not.
 var=val f
 echo "var=$var"

diff --git a/shell/ash_test/ash-vars/var_leak.right b/shell/ash_test/ash-vars/var_leak.right
index 01a5e3263ee38f5067d8f3e640fe5663973bfd2c..76468008629571df2808d9f6847d1723dfa940a4 100644 (file)
--- a/shell/ash_test/ash-vars/var_leak.right
+++ b/shell/ash_test/ash-vars/var_leak.right
@@ -1,4 +1,4 @@
 should be empty: ''
 should be empty: ''
 should be not empty: 'val2'
-should be not empty: 'val3'
+should be empty: ''

diff --git a/shell/ash_test/ash-vars/var_leak.tests b/shell/ash_test/ash-vars/var_leak.tests
index 5242e24eb19a2a5031bde9f96561db3c4262010f..adf66692ec64ffcc9c4ecb172cac655673f2d3d4 100755 (executable)
--- a/shell/ash_test/ash-vars/var_leak.tests
+++ b/shell/ash_test/ash-vars/var_leak.tests
@@ -15,9 +15,7 @@ VAR=''
 VAR=val2 exec 2>&1
 echo "should be not empty: '$VAR'"
 
-# ash follows the "function call is a special builtin" rule here
-# (bash does not do it)
 f() { true; }
 VAR=''
 VAR=val3 f
-echo "should be not empty: '$VAR'"
+echo "should be empty: '$VAR'"

diff --git a/shell/hush_test/hush-misc/env_and_func.tests b/shell/hush_test/hush-misc/env_and_func.tests
index 3efef1a41945e92f7f3f96033a793d5f9831ed1b..1c63eafd80270d1d39073273db336b2ccf96bcfd 100755 (executable)
--- a/shell/hush_test/hush-misc/env_and_func.tests
+++ b/shell/hush_test/hush-misc/env_and_func.tests
@@ -3,6 +3,6 @@ f() { echo "var=$var"; }
 # bash: POSIXLY_CORRECT behavior is to "leak" new variable values
 # out of function invocations (similar to "special builtins" behavior);
 # but in "bash mode", they don't leak.
-# hush does not "leak" values. ash does.
+# hush does not "leak" values. ash used to, but now does not.
 var=val f
 echo "var=$var"