From: Bart Schaefer <schaefer@brasslantern.com>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: [PATCH] local vs. nameref scoping (was Re: Up-scope named references, vs. ksh)
Date: Tue, 5 Mar 2024 21:04:27 -0800 [thread overview]
Message-ID: <CAH+w=7ZmAU64_o=gYQYjr-T9z79zmDSxwApz+9U2a7Rkqc0t=A@mail.gmail.com> (raw)
In-Reply-To: <20240305063059.detbg3orz57vmvwr@chazelas.org>
[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]
On Mon, Mar 4, 2024 at 9:43 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> On 3/5/24, Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > When a _named reference_ is created with 'typeset -n', all uses of PNAME
> > in assignments and expansions instead assign to or expand RNAME. This
> > also applies to 'unset PNAME' and to most subsequent uses of 'typeset'
> > with the exception of 'typeset -n' and 'typeset +n'
>
> Is this possible to change? I feel like if "typeset myvar" (or "local
> myvar") cannot be depended on to create a local parameter, a lot of
> code will no longer be safe that previously was (in the sense that it
> doesn't break if calling code / the shell environmnet has certain
> parameters defined).
Yes, it's possible, and the attached patch does so, with doc update
and new tests.
The division of labor among bin_typeset(), typeset_single(), and
createparam() is a bit hard to follow (as in, it's not really much of
a "division" at all).
On Mon, Mar 4, 2024 at 10:31 PM Stephane Chazelas <stephane@chazelas.org> wrote:
>
> See also:
>
> $ nameref action=PS1
> $ zmv -n '*' '$f.back'
> zsh: segmentation fault ./Src/zsh
A simpler reproducer:
% typeset -n foo=PS1
% () { local foo; foo=xx }
==1113679== Invalid read of size 8
==1113679== at 0x1A16E1: assignstrvalue (params.c:2684)
This patch fixes that, and one other potential crash that valgrind
complains about:
% typeset -n foo=bar
% typeset -n foo
==1113695== Invalid read of size 4
==1113695== at 0x136041: bin_typeset (builtin.c:3137)
I'll go ahead and push this since there doesn't seem to be any
argument about the change in function and it fixes two crash bugs.
[-- Attachment #2: local-of-nameref.txt --]
[-- Type: text/plain, Size: 4492 bytes --]
diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index 8c5e67e70..d179a0d1d 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -670,8 +670,9 @@ This manual was generated with Zsh tt(version()).
When a em(named reference) is created with `tt(typeset -n)', all uses
of var(pname) in assignments and expansions instead assign to or
expand var(rname). This also applies to `tt(unset )var(pname)' and to
-most subsequent uses of `tt(typeset)' with the exception of
-`tt(typeset -n)' and `tt(typeset +n)', so to remove a named reference,
+most subsequent uses of `tt(typeset)' with the exceptions of declaring
+a local in a called function, or updating a current-scope parameter with
+`tt(typeset -n)' or `tt(typeset +n)'. Thus to remove a named reference,
use either `tt(unset -n )var(pname)' (preferred) or one of:
ifzman()
example(tt(typeset -n )var(pname=)
diff --git a/Src/builtin.c b/Src/builtin.c
index 6f98990f9..829b899f8 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2030,11 +2030,10 @@ typeset_single(char *cname, char *pname, Param pm, int func,
int usepm, tc, keeplocal = 0, newspecial = NS_NONE, readonly, dont_set = 0;
char *subscript;
- if (pm && (pm->node.flags & PM_NAMEREF) && !((off|on) & PM_NAMEREF)) {
- if (!(off & PM_NAMEREF)) {
- if ((pm = (Param)resolve_nameref(pm, NULL)))
- pname = pm->node.nam;
- }
+ if (pm && (pm->node.flags & PM_NAMEREF) && !((off|on) & PM_NAMEREF) &&
+ (pm->level == locallevel || !(on & PM_LOCAL))) {
+ if ((pm = (Param)resolve_nameref(pm, NULL)))
+ pname = pm->node.nam;
if (pm && (pm->node.flags & PM_NAMEREF) &&
(on & ~(PM_NAMEREF|PM_LOCAL|PM_READONLY))) {
/* Changing type of PM_SPECIAL|PM_AUTOLOAD is a fatal error. *
@@ -3125,8 +3124,10 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
oldpm->u.str)
asg->value.scalar = dupstring(oldpm->u.str);
/* Defer read-only error to typeset_single() */
- if (!(hn->flags & PM_READONLY))
+ if (!(hn->flags & PM_READONLY)) {
unsetparam_pm(oldpm, 0, 1);
+ hn = NULL;
+ }
}
/* Passing a NULL pm to typeset_single() makes the
* nameref read-only before assignment, which breaks
@@ -3134,7 +3135,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
* so this is special-cased to permit that action
* like assign-at-create for other parameter types.
*/
- if (!(hn->flags & PM_READONLY))
+ if (hn && !(hn->flags & PM_READONLY))
hn = NULL;
}
}
diff --git a/Src/params.c b/Src/params.c
index 4bcf41c22..973df3fe5 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1034,7 +1034,8 @@ createparam(char *name, int flags)
}
if (oldpm && !(flags & PM_NAMEREF) &&
- (!(oldpm->node.flags & PM_RO_BY_DESIGN) || !(flags & PM_LOCAL)) &&
+ (oldpm->level == locallevel ?
+ !(oldpm->node.flags & PM_RO_BY_DESIGN) : !(flags & PM_LOCAL)) &&
(oldpm->node.flags & PM_NAMEREF) &&
(oldpm = upscope(oldpm, oldpm->base))) {
Param lastpm;
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index e45b922e2..bb0d11821 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -51,9 +51,19 @@
0:remove nameref attribute
>typeset ptr=var
- typeset -n ptr
- typeset -t ptr
- typeset -p ptr
+ typeset -n ptr=gvar
+ () {
+ local ptr
+ typeset -p ptr
+ }
+ typeset -p ptr
+0:Local non-reference hides outside reference
+>typeset ptr
+>typeset -n ptr=gvar
+
+ typeset -n ptr
+ typeset -t ptr
+ typeset -p ptr
0:change type of a placeholder
F:Other type changes are fatal errors, should this also be?
>typeset -n ptr=''
@@ -845,4 +855,39 @@ F:previously this could create an infinite recursion and crash
1:create nameref by pattern match not allowed
*?*typeset:1: invalid reference
+#
+# The following tests are run in interactive mode, using PS1 as an
+# assignable special with side-effects. This crashed at one time.
+#
+
+ # Note bypassing TYPESET_TO_UNSET here
+ $ZTST_testdir/../Src/zsh -fis <<<$'
+ typeset -n p=PS1
+ () {
+ typeset -p p
+ local p
+ typeset -p p
+ p=xx
+ typeset -p p
+ }
+ '
+0:regression: assign to local that shadows global named reference
+>typeset -g -n p=PS1
+>typeset p=''
+>typeset p=xx
+*?*
+
+ # Note bypassing TYPESET_TO_UNSET here
+ $ZTST_testdir/../Src/zsh -fis <<<$'
+ () {
+ typeset p=PS1
+ typeset -n p
+ p=zz
+ }
+ typeset -p PS1
+ '
+0:regression - converting a string into a named reference
+>typeset PS1=zz
+*?*
+
%clean
next prev parent reply other threads:[~2024-03-06 5:05 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-06 2:21 [PATCH 1/3]: Add named references Bart Schaefer
2023-02-08 3:45 ` Oliver Kiddle
2023-02-08 4:59 ` Bart Schaefer
2023-02-08 23:16 ` Bart Schaefer
2023-02-09 0:47 ` Oliver Kiddle
2023-02-09 2:01 ` Oliver Kiddle
2023-02-09 5:45 ` Bart Schaefer
2023-02-09 4:49 ` Bart Schaefer
2023-02-09 20:49 ` Oliver Kiddle
2023-02-09 23:07 ` Bart Schaefer
2023-02-11 3:04 ` Bart Schaefer
2023-02-11 3:55 ` Bart Schaefer
2023-02-11 5:36 ` Speaking of dangerous referents Bart Schaefer
2023-02-12 8:00 ` Oliver Kiddle
2023-02-12 8:34 ` Bart Schaefer
2023-02-11 7:02 ` [PATCH 1/3]: Add named references Oliver Kiddle
2023-02-11 7:45 ` Bart Schaefer
2023-02-11 23:43 ` Bart Schaefer
2023-02-11 23:45 ` Bart Schaefer
2023-02-12 7:38 ` Oliver Kiddle
2024-02-11 7:00 ` Stephane Chazelas
2024-02-11 16:14 ` Bart Schaefer
2024-02-11 16:42 ` Bart Schaefer
2024-02-18 3:26 ` Up-scope named references, vs. ksh Bart Schaefer
2024-02-20 21:05 ` Stephane Chazelas
2024-02-20 22:30 ` Bart Schaefer
2024-02-21 20:12 ` Stephane Chazelas
2024-02-29 5:16 ` Bart Schaefer
2024-03-01 18:22 ` Stephane Chazelas
2024-03-01 20:34 ` Bart Schaefer
2024-03-02 7:29 ` Bart Schaefer
2024-03-02 23:55 ` [PATCH] "typeset -nu" (was Re: Up-scope named references, vs. ksh) Bart Schaefer
2024-03-01 23:28 ` Up-scope named references, vs. ksh Bart Schaefer
2024-03-03 13:44 ` Stephane Chazelas
2024-03-03 19:04 ` Bart Schaefer
2024-03-03 20:27 ` Stephane Chazelas
2024-03-03 22:58 ` Bart Schaefer
2024-03-04 19:59 ` Stephane Chazelas
2024-03-05 1:05 ` Oliver Kiddle
2024-03-05 2:53 ` Bart Schaefer
2024-03-05 5:43 ` Mikael Magnusson
2024-03-05 6:30 ` Stephane Chazelas
2024-03-06 5:04 ` Bart Schaefer [this message]
2023-02-12 9:02 ` [PATCH 1/3]: Add named references Oliver Kiddle
2023-02-12 18:59 ` Bart Schaefer
2023-02-12 19:45 ` PM_* flags in zsh.h (was Re: [PATCH 1/3]: Add named references) Bart Schaefer
2023-02-12 21:01 ` Oliver Kiddle
2023-02-12 22:54 ` [PATCH 1/3]: Add named references Oliver Kiddle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAH+w=7ZmAU64_o=gYQYjr-T9z79zmDSxwApz+9U2a7Rkqc0t=A@mail.gmail.com' \
--to=schaefer@brasslantern.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).