From 3d962aacd5c86d18ad379eaca217bb53d0615a33 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Thu, 26 Nov 2015 12:43:08 -0800 Subject: 37229: non-local assignment to a parameter name whose outermost declaration is private, is an error rather than a silent no-op. Also fix %prep sed expression for Solaris. --- ChangeLog | 5 ++++ Src/Modules/param_private.c | 59 ++++++++++++++++++++++++++++++++++++--------- Test/V10private.ztst | 33 +++++++++++++++++++++---- 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 286e4b9f7..a1034cfcc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2015-11-26 Barton E. Schaefer + * 37229: Src/Modules/param_private.c, Test/V10private.ztst: non- + local assignment to a parameter name whose outermost declaration + is private, is an error rather than a silent no-op. Fix %prep + from 37225 for Solaris. + * unposted (cf. 37226): Test/B02typeset.ztst: export == typeset -xg * 37225: Test/V10private.ztst: fix test from 37208 diff --git a/Src/Modules/param_private.c b/Src/Modules/param_private.c index 7d1ba9c87..e13813c3d 100644 --- a/Src/Modules/param_private.c +++ b/Src/Modules/param_private.c @@ -170,11 +170,15 @@ static int bin_private(char *nam, char **args, LinkList assigns, Options ops, int func) { int from_typeset = 1; + int ofake = fakelevel; /* paranoia in case of recursive call */ makeprivate_error = 0; - if (!OPT_ISSET(ops, 'P')) - return bin_typeset(nam, args, assigns, ops, func); - else if (OPT_ISSET(ops, 'T')) { + if (!OPT_ISSET(ops, 'P')) { + fakelevel = 0; + from_typeset = bin_typeset(nam, args, assigns, ops, func); + fakelevel = ofake; + return from_typeset; + } else if (OPT_ISSET(ops, 'T')) { zwarn("bad option: -T"); return 1; } @@ -193,7 +197,7 @@ bin_private(char *nam, char **args, LinkList assigns, Options ops, int func) from_typeset = bin_typeset("private", args, assigns, ops, func); scanhashtable(paramtab, 0, 0, 0, makeprivate, 0); endparamscope(); - fakelevel = 0; + fakelevel = ofake; unqueue_signals(); return makeprivate_error | from_typeset; @@ -419,12 +423,21 @@ pph_unsetfn(Param pm, int explicit) * at this locallevel. Any that we find are marked PM_UNSET. If they are * already unset, they are also marked as PM_NORESTORE. * - * On exit from the scope, we find the same parameters again and remove + * The same game is played with PM_READONLY and PM_RESTRICTED, so private + * parameters are always read-only at deeper locallevel. This is possible + * because there is no builtin-command interface to set PM_RESTRICTED, so + * only parameters "known to the shell" can otherwise be restricted. + * + * On exit from the scope, we find the same parameters again and reset * the PM_UNSET and PM_NORESTORE flags as appropriate. We're guaraneed * by makeprivate() that PM_NORESTORE won't conflict with anything here. + * Similarly, PM_READONLY and PM_RESTRICTED are also reset. * */ +#define PM_WAS_UNSET PM_NORESTORE +#define PM_WAS_RONLY PM_RESTRICTED + static void scopeprivate(HashNode hn, int onoff) { @@ -432,17 +445,25 @@ scopeprivate(HashNode hn, int onoff) if (pm->level == locallevel) { if (!is_private(pm)) return; - if (onoff == PM_UNSET) + if (onoff == PM_UNSET) { if (pm->node.flags & PM_UNSET) - pm->node.flags |= PM_NORESTORE; + pm->node.flags |= PM_WAS_UNSET; else pm->node.flags |= PM_UNSET; - else { - if (pm->node.flags & PM_NORESTORE) + if (pm->node.flags & PM_READONLY) + pm->node.flags |= PM_WAS_RONLY; + else + pm->node.flags |= PM_READONLY; + } else { + if (pm->node.flags & PM_WAS_UNSET) pm->node.flags |= PM_UNSET; /* createparam() may frob */ else pm->node.flags &= ~PM_UNSET; - pm->node.flags &= ~PM_NORESTORE; + if (pm->node.flags & PM_WAS_RONLY) + pm->node.flags |= PM_READONLY; /* paranoia / symmetry */ + else + pm->node.flags &= ~PM_READONLY; + pm->node.flags &= ~(PM_WAS_UNSET|PM_WAS_RONLY); } } } @@ -478,8 +499,24 @@ getprivatenode(HashTable ht, const char *nam) HashNode hn = getparamnode(ht, nam); Param pm = (Param) hn; - while (!fakelevel && pm && locallevel > pm->level && is_private(pm)) + while (!fakelevel && pm && locallevel > pm->level && is_private(pm)) { + if (!(pm->node.flags & PM_UNSET)) { + /* + * private parameters are always marked PM_UNSET before we + * increment locallevel, so the only way we get here is + * when createparam() wants a new parameter that is not at + * the current locallevel and it has therefore cleared the + * PM_UNSET flag. + */ + DPUTS(pm->old, "BUG: PM_UNSET cleared in wrong scope"); + setfn_error(pm); + /* + * TODO: instead of throwing an error here, create a global + * parameter, insert as pm->old, handle WARN_CREATE_GLOBAL. + */ + } pm = pm->old; + } return (HashNode)pm; } diff --git a/Test/V10private.ztst b/Test/V10private.ztst index d5bee5e88..bb456f2ea 100644 --- a/Test/V10private.ztst +++ b/Test/V10private.ztst @@ -8,7 +8,7 @@ # Do not use .tmp here, ztst.zsh will remove it too soon (see %cleanup) mkdir private.TMP - sed '/^%prep/a \ + sed '/^%prep/a\ zmodload zsh/param/private' < $ZTST_srcdir/B02typeset.ztst > private.TMP/B02 %test @@ -243,15 +243,38 @@ F:note "typeset" rather than "private" in output from outer () { print X ${(kv)hash_test} hash_test=(even deeper) - array_test+=(${(kv)hash_test}) + { + array_test+=(${(kv)hash_test}) + } always { + print ${array_test-array_test not set} ${(t)array_test} + } } print Y ${(kv)hash_test} Z $array_test } print ${(kv)hash_test} ${(t)array_test} -0:privates are not visible in anonymous functions, part 3 +1:privates are not visible in anonymous functions, part 3 +>X top level +>array_test not set +?(anon):4: array_test: attempt to assign private in nested scope +F:future revision will create a global with this assignment + + typeset -a array_test + typeset -A hash_test=(top level) + () { + local -Pa array_test=(in function) + local -PA hash_test=($array_test) + () { + print X ${(kv)hash_test} + hash_test=(even deeper) + array_test+=(${(kv)hash_test}) + } + print Y ${(kv)hash_test} Z $array_test + } + print ${(kv)hash_test} $array_test +0:privates are not visible in anonymous functions, part 4 >X top level >Y in function Z in function ->even deeper +>even deeper even deeper typeset -A hash_test=(top level) () { @@ -263,7 +286,7 @@ F:note "typeset" rather than "private" in output from outer print Y ${(kv)hash_test} } print ${(t)hash_test} ${(kv)hash_test} -0:privates are not visible in anonymous functions, part 4 +0:privates are not visible in anonymous functions, part 5 >X top level >Y in function > -- cgit 1.4.1