klibc - Mar 2020 - [klibc:update-dash] dash: eval: Variable assignments on functions are no longer persistent

Commit-ID:  4f8448353f3b894eac522461109df97ea6af6a54
Gitweb:    
http://git.kernel.org/?p=libs/klibc/klibc.git;a=commit;h=4f8448353f3b894eac522461109df97ea6af6a54
Author:     Herbert Xu <herbert at gondor.apana.org.au>
AuthorDate: Wed, 4 Apr 2018 17:54:01 +0800
Committer:  Ben Hutchings <ben at decadent.org.uk>
CommitDate: Sat, 28 Mar 2020 21:42:55 +0000

[klibc] dash: eval: Variable assignments on functions are no longer persistent

[ dash commit 3cd57b6ed933c299e4d7c3c1e8390194635bbab8 ]

Dirk Fieldhouse <fieldhouse at 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 at gmx.net>
Signed-off-by: Herbert Xu <herbert at gondor.apana.org.au>
Signed-off-by: Ben Hutchings <ben at decadent.org.uk>

---
 usr/dash/eval.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/usr/dash/eval.c b/usr/dash/eval.c
index e28e56cb..722066e5 100644
--- a/usr/dash/eval.c
+++ b/usr/dash/eval.c
@@ -886,7 +886,6 @@ raise:
 		break;
 
 	case CMDFUNCTION:
-		poplocalvars(1);
 		if (evalfun(cmdentry.u.func, argc, argv, flags))
 			goto raise;
 		break;
@@ -971,9 +970,7 @@ evalfun(struct funcnode *func, int argc, char **argv, int
flags)
 	shellparam.p = argv + 1;
 	shellparam.optind = 1;
 	shellparam.optoff = -1;
-	pushlocalvars();
 	evaltree(func->n.ndefun.body, flags & EV_TESTED);
-	poplocalvars(0);
 funcdone:
 	INTOFF;
 	loopnest = saveloopnest;