"functions -c" and TRAPxxx

"functions -c" and TRAPxxx

[Bart Schaefer]

I'm not feeling quite knowledgeable enough about settrap /
removetrapnode vis-a-vis reference counting to offer a patch for this
yet, so if someone else wants to jump in or educate me, please do.

Creating a TRAPxxx function by "aliasing" to another function with
"functions -c" does not create the expected trap handler.

% TRAPUSR1() { print $0 }
% functions -c TRAPUSR1 TRAPUSR2
% trap
TRAPUSR1 () {
    print $0
}
% functions -m TRAP\*
TRAPUSR1 () {
    print $0
}
TRAPUSR2 () {
    print $0
}
%

Aside:  Local trap handling in _main_complete confuses trap-name completion:

% compinit
% zstyle '*' format %d
% trap
% functions -c TRAP<TAB>
old name
TRAPINT   TRAPQUIT

I don't know that we can do anything about this, I just found it
briefly confusing.

Re: "functions -c" and TRAPxxx

[Peter Stephenson]

> On 07 June 2022 at 02:59 Bart Schaefer <schaefer@brasslantern.com> wrote:
> I'm not feeling quite knowledgeable enough about settrap /
> removetrapnode vis-a-vis reference counting to offer a patch for this
> yet, so if someone else wants to jump in or educate me, please do.
> 
> Creating a TRAPxxx function by "aliasing" to another function with
> "functions -c" does not create the expected trap handler.

I don't think there's any magic for this, it'll have to be ad hoc
calls to settrap and unsettrap based on the function name.

pws

Re: "functions -c" and TRAPxxx

[Bart Schaefer]

On Tue, Jun 7, 2022 at 2:59 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > Creating a TRAPxxx function by "aliasing" to another function with
> > "functions -c" does not create the expected trap handler.
>
> I don't think there's any magic for this, it'll have to be ad hoc
> calls to settrap and unsettrap based on the function name.

Yes, but e.g. in exec.c there is a comment
                /*
                 * Remove the old node explicitly in case it has
                 * an alternative name
                 */
I know most of what's going on there, but "functions -c" is creating
an alternate name, and there is also the failure case of settrap()
returning nonzero to handle.

Re: "functions -c" and TRAPxxx

[Bart Schaefer]

On Tue, Jun 7, 2022 at 7:57 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> I know most of what's going on there, but "functions -c" is creatin
> an alternate name, and there is also the failure case of settrap()
> returning nonzero to handle.

The following works for the case of settrap() succeeding, but I'm not
sure how to force settrap() to fail.

diff --git a/Src/builtin.c b/Src/builtin.c
index 1cef7cce8..7f00d9d29 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3274,6 +3274,7 @@ bin_functions(char *name, char **argv, Options
ops, int func)

     if (OPT_ISSET(ops,'c')) {
     Shfunc newsh;
+    char *s = argv[1];
     if (!*argv || !argv[1] || argv[2]) {
         zwarnnam(name, "-c: requires two arguments");
         return 1;
@@ -3305,7 +3306,21 @@ bin_functions(char *name, char **argv, Options
ops, int func)
         newsh->redir->nref++;
     if (shf->sticky)
         newsh->sticky = sticky_emulation_dup(sticky, 0);
-    shfunctab->addnode(shfunctab, ztrdup(argv[1]), &newsh->node);
+    /* is newsh a signal trap? (adapted from exec.c) */
+    if (!strncmp(s, "TRAP", 4)) {
+        int signum = getsignum(s + 4);
+        if (signum != -1) {
+        if (settrap(signum, NULL, ZSIG_FUNC)) {
+            freeeprog(newsh->funcdef);
+            dircache_set(&newsh->filename, NULL);
+            zfree(newsh, sizeof(*newsh));
+            return 1;
+        }
+        /* Remove any old node explicitly */
+        removetrapnode(signum);
+        }
+    }
+    shfunctab->addnode(shfunctab, ztrdup(s), &newsh->node);
     return 0;
     }

Re: "functions -c" and TRAPxxx

[Peter Stephenson]

> On 09 June 2022 at 04:24 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Tue, Jun 7, 2022 at 7:57 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > I know most of what's going on there, but "functions -c" is creatin
> > an alternate name, and there is also the failure case of settrap()
> > returning nonzero to handle.
> 
> The following works for the case of settrap() succeeding, but I'm not
> sure how to force settrap() to fail.

Looks entirely plausible; given TRAP{INT,QUIT} already work this ought
to work too.

Looks like the only way for settrap to fail is using a signal number
that's not handled interactively.

pws

Re: "functions -c" and TRAPxxx

[Bart Schaefer]

On Thu, Jun 9, 2022 at 1:56 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> Looks like the only way for settrap to fail is using a signal number
> that's not handled interactively.

% trapper() { print $0 }
% for t in $(kill -l)
for> do functions -c trapper TRAP$t || echo cannot $t
for> done
%

So as far as I can tell it's not possible for settrap() to fail (on
ubuntu, anyway) if getsignum() succeeds.

Some of those traps are ignored (STOP, KILL) but you can set them anyway.

Re: "functions -c" and TRAPxxx

[Bart Schaefer]

On Thu, Jun 9, 2022 at 8:19 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> So as far as I can tell it's not possible for settrap() to fail (on
> ubuntu, anyway) if getsignum() succeeds.

Hmm, I have a different result now, so I must have ... run the wrong
build, before?

% for x in $(kill -l); do
for> { functions -c trapper TRAP$x } always { TRY_BLOCK_ERROR=0 }
for> done
zsh: can't trap SIGTSTP in interactive shells
zsh: can't trap SIGTTIN in interactive shells
zsh: can't trap SIGTTOU in interactive shells

Interesting that failing to set a trap is a hard error condition.