* exporting environment variables to shell functions (maybe bug)
@ 2010-09-03 8:34 Joke de Buhr
2010-09-03 8:54 ` Peter Stephenson
0 siblings, 1 reply; 3+ messages in thread
From: Joke de Buhr @ 2010-09-03 8:34 UTC (permalink / raw)
To: zsh-users
[-- Attachment #1: Type: Text/Plain, Size: 874 bytes --]
I've ask this question a couple of days before but I didn't get any response
[1]. I think this may be a bug.
If I define a shell function like this one:
do_print() {
print "__${NAME}__" # print environment variable "NAME"
}
and later call the function while setting the environment variable "NAME"
NAME="hello" do_print
the output is "__hello__" as everybody would expect. But if the function is
called this way
NAME="hello" # set NAME (no export)
NAME="::${NAME}::" do_print # export something as NAME containing $NAME
the output is "__::::__" instead of "__::hello::__". If the same think is done
using a command instead of a shell function the output is "__::hello::__".
Refer to the older email [1] for a listing of different test cases.
[1] http://www.zsh.org/mla/users/2010/msg00644.html
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 706 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: exporting environment variables to shell functions (maybe bug)
2010-09-03 8:34 exporting environment variables to shell functions (maybe bug) Joke de Buhr
@ 2010-09-03 8:54 ` Peter Stephenson
2010-09-03 9:52 ` Joke de Buhr
0 siblings, 1 reply; 3+ messages in thread
From: Peter Stephenson @ 2010-09-03 8:54 UTC (permalink / raw)
To: zsh-users
On Fri, 3 Sep 2010 10:34:30 +0200
Joke de Buhr <joke@seiken.de> wrote:
> I've ask this question a couple of days before but I didn't get any
> response [1]. I think this may be a bug.
It's a bug, so I sent a fix to zsh-workers, not realising you weren't
on that list... zsh-workers/28220
pws
On Tue, 31 Aug 2010 17:54:25 +0200
Joke de Buhr <joke@seiken.de> wrote:
> ## 4: HELLO not exported to shell function, different from 2 and 3
> $ call() { env | grep '^HELLO' }
> $ HELLO=world
> $ HELLO=$HELLO call
> HELLO= ## <-- empty
(Moved to zsh-workers immediately, this time.)
That's a bug. When HELLO needs to be restored explicitly because the
code where it's set to something else is running in the main shell, we
remove the parameter from the table to be restored later. The
assignment than can't see the value it's temporarily modifying. You
don't need to search the environment for this; zsh consistently screws
up the parameter within the shell, too.
I think we can just copy the parameter instead of removing it, so long
as we copy it properly.
valgrind says this doesn't cause any leaks.
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.182
diff -p -u -r1.182 exec.c
--- Src/exec.c 22 Aug 2010 20:08:57 -0000 1.182
+++ Src/exec.c 31 Aug 2010 19:11:41 -0000
@@ -3313,13 +3313,17 @@ save_params(Estate state, Wordcode pc, L
while (wc_code(ac = *pc) == WC_ASSIGN) {
s = ecrawstr(state->prog, pc + 1, NULL);
if ((pm = (Param) paramtab->getnode(paramtab, s))) {
+ Param tpm;
if (pm->env)
delenv(pm);
if (!(pm->node.flags & PM_SPECIAL)) {
- paramtab->removenode(paramtab, s);
+ tpm = (Param) zshcalloc(sizeof *tpm);
+ tpm->node.nam = ztrdup(pm->node.nam);
+ copyparam(tpm, pm, 0);
+ pm = tpm;
} else if (!(pm->node.flags & PM_READONLY) &&
(unset(RESTRICTED) || !(pm->node.flags & PM_RESTRICTED))) {
- Param tpm = (Param) hcalloc(sizeof *tpm);
+ tpm = (Param) hcalloc(sizeof *tpm);
tpm->node.nam = pm->node.nam;
copyparam(tpm, pm, 1);
pm = tpm;
@@ -3383,8 +3387,9 @@ restore_params(LinkList restorelist, Lin
break;
}
pm = tpm;
- } else
+ } else {
paramtab->addnode(paramtab, pm->node.nam, pm);
+ }
if ((pm->node.flags & PM_EXPORTED) && ((s = getsparam(pm->node.nam))))
addenv(pm, s);
}
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.161
diff -p -u -r1.161 params.c
--- Src/params.c 3 Jun 2010 13:38:17 -0000 1.161
+++ Src/params.c 31 Aug 2010 19:11:41 -0000
@@ -927,11 +927,17 @@ createspecialhash(char *name, GetNodeFun
}
-/* Copy a parameter */
+/*
+ * Copy a parameter
+ *
+ * If fakecopy is set, we are just saving the details of a special
+ * parameter. Otherwise, the result will be used as a real parameter
+ * and we need to do more work.
+ */
/**/
void
-copyparam(Param tpm, Param pm, int toplevel)
+copyparam(Param tpm, Param pm, int fakecopy)
{
/*
* Note that tpm, into which we're copying, may not be in permanent
@@ -942,7 +948,8 @@ copyparam(Param tpm, Param pm, int tople
tpm->node.flags = pm->node.flags;
tpm->base = pm->base;
tpm->width = pm->width;
- if (!toplevel)
+ tpm->level = pm->level;
+ if (!fakecopy)
tpm->node.flags &= ~PM_SPECIAL;
switch (PM_TYPE(pm->node.flags)) {
case PM_SCALAR:
@@ -963,13 +970,15 @@ copyparam(Param tpm, Param pm, int tople
break;
}
/*
- * If called from inside an associative array, that array is later going
- * to be passed as a real parameter, so we need the gets and sets
- * functions to be useful. However, the saved associated array is
- * not itself special, so we just use the standard ones.
- * This is also why we switch off PM_SPECIAL.
+ * If the value is going to be passed as a real parameter (e.g. this is
+ * called from inside an associative array), we need the gets and sets
+ * functions to be useful.
+ *
+ * In this case we assume the the saved parameter is not itself special,
+ * so we just use the standard functions. This is also why we switch off
+ * PM_SPECIAL.
*/
- if (!toplevel)
+ if (!fakecopy)
assigngetset(tpm);
}
Index: Test/A06assign.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A06assign.ztst,v
retrieving revision 1.5
diff -p -u -r1.5 A06assign.ztst
--- Test/A06assign.ztst 23 Sep 2006 06:55:29 -0000 1.5
+++ Test/A06assign.ztst 31 Aug 2010 19:11:41 -0000
@@ -277,3 +277,18 @@
>
>
>
+
+ call() { print $HELLO; }
+ export HELLO=world
+ call
+ HELLO=universe call
+ call
+ HELLO=${HELLO}liness call
+ call
+ unset HELLO
+0:save and restore when using original value in temporary
+>world
+>universe
+>world
+>worldliness
+>world
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: exporting environment variables to shell functions (maybe bug)
2010-09-03 8:54 ` Peter Stephenson
@ 2010-09-03 9:52 ` Joke de Buhr
0 siblings, 0 replies; 3+ messages in thread
From: Joke de Buhr @ 2010-09-03 9:52 UTC (permalink / raw)
To: zsh-users
[-- Attachment #1: Type: Text/Plain, Size: 5890 bytes --]
On Friday 03 September 2010 10:54:44 Peter Stephenson wrote:
> On Fri, 3 Sep 2010 10:34:30 +0200
>
> Joke de Buhr <joke@seiken.de> wrote:
> > I've ask this question a couple of days before but I didn't get any
> > response [1]. I think this may be a bug.
>
> It's a bug, so I sent a fix to zsh-workers, not realising you weren't
> on that list... zsh-workers/28220
It's only important it will be fixed. This bug caused some trouble in one of
my scripts and it took some time to realize the identical variable name was to
blame.
> pws
>
>
> On Tue, 31 Aug 2010 17:54:25 +0200
>
> Joke de Buhr <joke@seiken.de> wrote:
> > ## 4: HELLO not exported to shell function, different from 2 and 3
> > $ call() { env | grep '^HELLO' }
> > $ HELLO=world
> > $ HELLO=$HELLO call
> >
> > HELLO= ## <-- empty
>
> (Moved to zsh-workers immediately, this time.)
>
> That's a bug. When HELLO needs to be restored explicitly because the
> code where it's set to something else is running in the main shell, we
> remove the parameter from the table to be restored later. The
> assignment than can't see the value it's temporarily modifying. You
> don't need to search the environment for this; zsh consistently screws
> up the parameter within the shell, too.
>
> I think we can just copy the parameter instead of removing it, so long
> as we copy it properly.
>
> valgrind says this doesn't cause any leaks.
>
> Index: Src/exec.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
> retrieving revision 1.182
> diff -p -u -r1.182 exec.c
> --- Src/exec.c 22 Aug 2010 20:08:57 -0000 1.182
> +++ Src/exec.c 31 Aug 2010 19:11:41 -0000
> @@ -3313,13 +3313,17 @@ save_params(Estate state, Wordcode pc, L
> while (wc_code(ac = *pc) == WC_ASSIGN) {
> s = ecrawstr(state->prog, pc + 1, NULL);
> if ((pm = (Param) paramtab->getnode(paramtab, s))) {
> + Param tpm;
> if (pm->env)
> delenv(pm);
> if (!(pm->node.flags & PM_SPECIAL)) {
> - paramtab->removenode(paramtab, s);
> + tpm = (Param) zshcalloc(sizeof *tpm);
> + tpm->node.nam = ztrdup(pm->node.nam);
> + copyparam(tpm, pm, 0);
> + pm = tpm;
> } else if (!(pm->node.flags & PM_READONLY) &&
> (unset(RESTRICTED) || !(pm->node.flags & PM_RESTRICTED))) {
> - Param tpm = (Param) hcalloc(sizeof *tpm);
> + tpm = (Param) hcalloc(sizeof *tpm);
> tpm->node.nam = pm->node.nam;
> copyparam(tpm, pm, 1);
> pm = tpm;
> @@ -3383,8 +3387,9 @@ restore_params(LinkList restorelist, Lin
> break;
> }
> pm = tpm;
> - } else
> + } else {
> paramtab->addnode(paramtab, pm->node.nam, pm);
> + }
> if ((pm->node.flags & PM_EXPORTED) && ((s =
> getsparam(pm->node.nam)))) addenv(pm, s);
> }
> Index: Src/params.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/params.c,v
> retrieving revision 1.161
> diff -p -u -r1.161 params.c
> --- Src/params.c 3 Jun 2010 13:38:17 -0000 1.161
> +++ Src/params.c 31 Aug 2010 19:11:41 -0000
> @@ -927,11 +927,17 @@ createspecialhash(char *name, GetNodeFun
> }
>
>
> -/* Copy a parameter */
> +/*
> + * Copy a parameter
> + *
> + * If fakecopy is set, we are just saving the details of a special
> + * parameter. Otherwise, the result will be used as a real parameter
> + * and we need to do more work.
> + */
>
> /**/
> void
> -copyparam(Param tpm, Param pm, int toplevel)
> +copyparam(Param tpm, Param pm, int fakecopy)
> {
> /*
> * Note that tpm, into which we're copying, may not be in permanent
> @@ -942,7 +948,8 @@ copyparam(Param tpm, Param pm, int tople
> tpm->node.flags = pm->node.flags;
> tpm->base = pm->base;
> tpm->width = pm->width;
> - if (!toplevel)
> + tpm->level = pm->level;
> + if (!fakecopy)
> tpm->node.flags &= ~PM_SPECIAL;
> switch (PM_TYPE(pm->node.flags)) {
> case PM_SCALAR:
> @@ -963,13 +970,15 @@ copyparam(Param tpm, Param pm, int tople
> break;
> }
> /*
> - * If called from inside an associative array, that array is later
> going - * to be passed as a real parameter, so we need the gets and
> sets - * functions to be useful. However, the saved associated array
> is - * not itself special, so we just use the standard ones.
> - * This is also why we switch off PM_SPECIAL.
> + * If the value is going to be passed as a real parameter (e.g. this
> is + * called from inside an associative array), we need the gets and
> sets + * functions to be useful.
> + *
> + * In this case we assume the the saved parameter is not itself
> special, + * so we just use the standard functions. This is also why
> we switch off + * PM_SPECIAL.
> */
> - if (!toplevel)
> + if (!fakecopy)
> assigngetset(tpm);
> }
>
> Index: Test/A06assign.ztst
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Test/A06assign.ztst,v
> retrieving revision 1.5
> diff -p -u -r1.5 A06assign.ztst
> --- Test/A06assign.ztst 23 Sep 2006 06:55:29 -0000 1.5
> +++ Test/A06assign.ztst 31 Aug 2010 19:11:41 -0000
> @@ -277,3 +277,18 @@
>
>
>
> +
> + call() { print $HELLO; }
> + export HELLO=world
> + call
> + HELLO=universe call
> + call
> + HELLO=${HELLO}liness call
> + call
> + unset HELLO
> +0:save and restore when using original value in temporary
> +>world
> +>universe
> +>world
> +>worldliness
> +>world
>
>
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and
> Wales, registered number 4187346, registered office Churchill House,
> Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 706 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-09-03 9:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-03 8:34 exporting environment variables to shell functions (maybe bug) Joke de Buhr
2010-09-03 8:54 ` Peter Stephenson
2010-09-03 9:52 ` Joke de Buhr
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).