Re: [PATCH 4/4] [VAR] Replace cmdenviron with localvars
On Thu, May 27, 2010 at 01:51:41PM +1000, Herbert Xu wrote: > In order to ensure that local still works, it is now a special > built-in. Although there are reasons to do this (a 'local' function containing 'command local' would not work, and export, readonly and ksh93's typeset are special as well), consensus seems to be for local to be a regular builtin. posh's local was special for a short time, but this was then reverted. Nevertheless, it is very nice to have IFS=: read ... working again. -- Jilles Tjoelker -- To unsubscribe from this list: send the line "unsubscribe dash" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] [VAR] Replace cmdenviron with localvars
On Thu, May 27, 2010 at 12:37:23AM +0200, Jilles Tjoelker wrote:
>
> This change breaks passing variable assignments to regular builtins.
> For example,
> dash -c 'HOME=/ cd; pwd'
> This should print /. (For some reason, this does not work in ksh93
> either.)
Thanks, I have fixed it as follows.
commit 1d806ac1fbafb867f6252e184e1be05c0829ab71
Author: Herbert Xu
Date: Thu May 27 11:50:19 2010 +0800
[VAR] Do not poplocalvars prematurely on regular utilities
The recent cmdenviron removal broke regular utilities by calling
poplocalvars too early. This patch fixes that by postponing the
poplocalvars for regular utilities until they have completed.
In order to ensure that local still works, it is now a special
built-in.
Signed-off-by: Herbert Xu
diff --git a/ChangeLog b/ChangeLog
index eb12538..4fc35a6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,7 @@
2010-05-27 Herbert Xu
* Fix poplocalvar on abnormal exit from function.
+ * Do not poplocalvars prematurely on regular utilities.
2010-05-26 Herbert Xu
diff --git a/src/builtins.def.in b/src/builtins.def.in
index 266d0ec..4441fe4 100644
--- a/src/builtins.def.in
+++ b/src/builtins.def.in
@@ -71,7 +71,7 @@ falsecmd -u false
getoptscmd -u getopts
hashcmdhash
jobscmd-u jobs
-localcmd -a local
+localcmd -as local
printfcmd printf
pwdcmd pwd
readcmd-u read
diff --git a/src/eval.c b/src/eval.c
index 2cd931b..337667f 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -847,9 +847,11 @@ bail:
/* NOTREACHED */
case CMDBUILTIN:
- poplocalvars(spclbltin > 0 || argc == 0);
- if (execcmd && argc > 1)
- listsetvar(varlist.list, VEXPORT);
+ if (spclbltin > 0 || argc == 0) {
+ poplocalvars(1);
+ if (execcmd && argc > 1)
+ listsetvar(varlist.list, VEXPORT);
+ }
if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) {
int status;
int i;
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~}
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] [VAR] Replace cmdenviron with localvars
On Wed, May 26, 2010 at 09:00:34PM +1000, Herbert Xu wrote:
> [VAR] Replace cmdenviron with localvars
> This patch replaces the cmdenviron mechanism for temporary command
> variables with the localvars mechanism used by functions.
> This reduces code size, and more importantly, makes the variable
> assignment take effect immediately as required by POSIX.
This change breaks passing variable assignments to regular builtins.
For example,
dash -c 'HOME=/ cd; pwd'
This should print /. (For some reason, this does not work in ksh93
either.)
Or
dash -c 'cd /bin&&CDPATH=/ cd bin; pwd'
This should print /bin twice and not fail any command. (Again this fails
in ksh93.)
A more common example,
IFS=: read -r x y z
is already broken in master before this change (due to git commit
55c46b7286f5d9f2d8291158203e2b61d2494420 which effectively replaced
bltinlookup("IFS") with ifsval()).
Similar issues may pop up with
PATH=/foo command -V bar
PATH=/foo type bar
LC_NUMERIC=de_DE.UTF-8 printf '%f\n' 1,2
(Note: the latter is broken in bash, but in ksh93 and zsh it works, as
well as with an external printf(1) program.)
'local' may need additional magic to avoid making things local to the
temporary regular-builtin scope. I suppose only functions should induce
scope for 'local', such that things like command eval 'local i' continue
to create a variable local to the function.
--
Jilles Tjoelker
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
