help / color / mirror / code / Atom feed
From: Oliver Kiddle <opk@zsh.org>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: [PATCH 1/3]: Add named references
Date: Sat, 11 Feb 2023 08:02:05 +0100	[thread overview]
Message-ID: <40726-1676098925.110777@U2kb.d0Pd.I9ml> (raw)
In-Reply-To: <CAH+w=7YVJO-HkneMpnfBbqBztPaXdXTD=mo-vHbdUW00TiFVBQ@mail.gmail.com>

Bart Schaefer wrote:
> Closing in on this, I think ... remaining decisions on which input
> would be appreciated:
> On Thu, Feb 9, 2023 at 3:07 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > > Ksh prints "global reference cannot refer to local variable".
> >
> > At what point does that happen?  Upon the assignment ref=var ?
> At the moment, just toying with this, I have it erroring when
> EMULATE_KSH, printing a warning when WARN_NESTED_VAR, and otherwise
> silently creating the down-scope reference.  Thoughts?  Perhaps
> WARN_CREATE_GLOBAL would actually be better here, or maybe check for
> either one.

I'd make it a fatal error unconditionally. I don't like it if the target
of a nameref can switch to something else following a return. I'd be
fine with the reference becoming unset at the time of the return if it
refers to a variable that is going out of scope. Can that be done as
part of the same code that drops the local variables? (I'm guessing it
is easy for local but hard for private)

The mention of WARN_NESTED_VAR raises something else. Surely a reference
should disable that warning:

    toplevel() {
      local foo="in fn"
    nested() {
      typeset -n ref=foo
      ref="in nested"
    setopt warn_nested_var
    nested:2: scalar parameter foo set in enclosing scope in function nested

> > Relatedly, what should happen on any failed assignment?  E.g.
> >
> > typeset -n xy yx
> > xy=yx  # OK so far
> > yx=xy  # Oops, invalid loop
> >
> > Should yx become unset, or should it remain a nameref with no referent?
> This probably doesn't matter except at the command line as the
> referent loop is otherwise fatal.

Current code even allows:

  typeset -n ref

I'd suggest that we need the self-reference check whenever assigning to
a reference. But that entails evaluation of a subscript.

> > > When relying only on dynamic scoping, it would be good practice to
> > > define all the namerefs to passed parameters as early as possible in a
> > > function to reduce the risk of a name clash.
> >
> > If you were going to put that in the doc, where would it go?

Do we to put recommended good practices in the doc at all as opposed to
it being a terse description of the functionality. I'd prefer it out of
the way with examples rather than adding fluff to the places you list.
I'd also consider it bad practice not to always assign a reference when
creating it.

> > > It might deprive us of many clever tricks but parse_subscript() could
> > > gain a flag to disable anything questionable like command substitution
> > > and math evaluation side-effects.
> parse_subscript() is already a bit of a hack, it sets up its own
> context and invokes the lexer.  A flag such as that would, I think,
> require plugging in an alternate lexer, hence my question about doing
> a simpler examination of the string.

That's not as robust but certainly an option. The temporary use of
NOEXEC that you mention in the other message certainly goes a long way
to alleviating security concerns.


  parent reply	other threads:[~2023-02-11  7:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-06  2:21 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               ` Oliver Kiddle [this message]
2023-02-11  7:45                 ` [PATCH 1/3]: Add named references Bart Schaefer
2023-02-11 23:43                   ` Bart Schaefer
2023-02-11 23:45                     ` Bart Schaefer
2023-02-12  7:38                     ` Oliver Kiddle
2023-02-12  9:02             ` 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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40726-1676098925.110777@U2kb.d0Pd.I9ml \
    --to=opk@zsh.org \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \


* 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


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).