Re: Feature request: ZSH_XTRACEFD variable

[Timothée Mazzucotelli <timothee.mazzucotelli@xxxxxxxxx>]

Thank you Peter for your hints.

Here is a first draft:

diff --git a/Src/exec.c b/Src/exec.c
index 2acb2c0bc..5e550cb24 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5393,7 +5393,7 @@ execshfunc(Shfunc shf, LinkList args)
     cmdsp = 0;
     if ((osfc = sfcontext) == SFC_NONE)
  sfcontext = SFC_DIRECT;
-    xtrerr = stderr;
+    xtrerr = fdopen(zsh_xtracefd, "w");

     doshfunc(shf, args, 0);

diff --git a/Src/init.c b/Src/init.c
index 445cd3937..396f13556 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -609,8 +609,10 @@ init_io(char *cmd)
  SHTTY = -1;
     }

-    /* Send xtrace output to stderr -- see execcmd() */
-    xtrerr = stderr;
+    /* Send xtrace output to zsh_xtracefd file descriptor -- see execcmd()
*/
+    if (zsh_xtracefd == 0)
+      zsh_xtracefd = 2;
+    xtrerr = fdopen(zsh_xtracefd, "w");

     /* Make sure the tty is opened read/write. */
     if (isatty(0)) {
diff --git a/Src/params.c b/Src/params.c
index 1499e3a40..5e3f5cdbf 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -102,7 +102,8 @@ zlong lastval, /* $?           */
      zterm_lines, /* $LINES       */
      rprompt_indent, /* $ZLE_RPROMPT_INDENT */
      ppid, /* $PPID        */
-     zsh_subshell; /* $ZSH_SUBSHELL */
+     zsh_subshell, /* $ZSH_SUBSHELL */
+     zsh_xtracefd;  /* $ZSH_XTRACEFD */

 /* $FUNCNEST    */
 /**/
@@ -264,6 +265,9 @@ static const struct gsu_array pipestatus_gsu =
 static const struct gsu_integer rprompt_indent_gsu =
 { intvargetfn, zlevarsetfn, rprompt_indent_unsetfn };

+static const struct gsu_integer xtracefd_gsu =
+{ intvargetfn, xtracefdsetfn, xtracefdunsetfn };
+
 /* Nodes for special parameters for parameter hash table */

 #ifdef HAVE_UNION_INIT
@@ -353,6 +357,7 @@ IPDEF5("LINES", &zterm_lines, zlevar_gsu),
 IPDEF5U("ZLE_RPROMPT_INDENT", &rprompt_indent, rprompt_indent_gsu),
 IPDEF5("SHLVL", &shlvl, varinteger_gsu),
 IPDEF5("FUNCNEST", &zsh_funcnest, varinteger_gsu),
+IPDEF5("ZSH_XTRACEFD", &zsh_xtracefd, xtracefd_gsu),

 /* Don't import internal integer status variables. */
 #define IPDEF6(A,B,F)
{{NULL,A,PM_INTEGER|PM_SPECIAL|PM_DONTIMPORT},BR((void
*)B),GSU(F),10,0,NULL,NULL,NULL,0}
@@ -4387,6 +4392,45 @@ setsecondstype(Param pm, int on, int off)
     return 0;
 }

+/* Function to set value of special parameter `ZSH_XTRACEFD' */
+
+/**/
+void
+xtracefdsetfn(Param pm, zlong fd)
+{
+    int current_fd;
+
+    /* Check that the given file descriptor is valid */
+    if (fcntl(fd, F_GETFD) != -1 || errno != EBADF) {
+      current_fd = intvargetfn(pm);
+      /* We never close file descriptors 0 (stdin), 1 (stdout) or 2
(stderr) */
+      if (current_fd > 2) {
+        close(current_fd);
+        fclose(xtrerr);
+      }
+      intvarsetfn(pm, fd);
+      xtrerr = fdopen(fd, "w");
+    } else
+      zwarn("file descriptor %d is not valid", fd);
+
+}
+
+/* Function to unset value of special parameter `ZSH_XTRACEFD' */
+
+/**/
+void
+xtracefdunsetfn(Param pm, UNUSED(int exp))
+{
+    int current_fd = intvargetfn(pm);
+    if (current_fd == 2)  /* Nothing to do, already using stderr */
+      return;
+    else {  /* Reset to file descriptor 2 (stderr) */
+      intvarsetfn(pm, 2);
+      if (current_fd > 1) fclose(xtrerr);  /* Never close stdin or stdout
*/
+      xtrerr = stderr;
+    }
+}
+
 /* Function to get value for special parameter `USERNAME' */

 /**/
diff --git a/Src/utils.c b/Src/utils.c
index 46cf7bcf6..93ea8044f 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1765,7 +1765,7 @@ void
 printprompt4(void)
 {
     if (!xtrerr)
- xtrerr = stderr;
+ xtrerr = fdopen(zsh_xtracefd, "w");
     if (prompt4) {
  int l, t = opts[XTRACE];
  char *s = dupstring(prompt4);


And some notes:

There is one place where I didn't change the code, because I wasn't sure it
was needed:


> Doing this without creating leaks or crashes on bad file descriptors
could be interesting.

About leaks, I think file pointers are all freed in the params.c set and
unset functions.
However I'm not sure about exec.c, line 5396, in function execshfunc, where
xtrerr was
previously always reassigned to stderr. Now it is reassigned to a new file
pointer obtained
with fdopen(zsh_xtracefd, "w") each time, and maybe this could cause memory
leaks.

About crashes on bad file descriptors, I used if (fcntl(fd, F_GETFD) != -1
|| errno != EBADF) as a condition
to test the validity of a file descriptor, but I'm not sure it can handle
all the cases, or if it even makes
sense. It's working with basic cases though.




On Sun, Jul 21, 2019 at 5:23 PM Peter Stephenson <p.stephenson@xxxxxxxxxxx>
wrote:

> On Sun, 2019-07-21 at 17:08 +0200, Timothée Mazzucotelli wrote:
> > I'm willing to help implementing this feature (equivalent of Bash's
> > BASH_XTRACEFD). Any advice where I should start? Who I should get in
> touch
> > with?
>
> This is the right place.  Certainly feel free to investigate.
>
> Have a look at the way the FILE *xtrerr is handed in the source code.  This
> is going to need some more careful management.  Doing this without creating
> leaks or crashes on bad file descriptors could be interesting.
> In most places, once it is reopened to a different file the redirection
> should "just work", but there maybe some interesting special cases
> as we do do slightly funny things with xtrerr at some points.
>
> I think you're going to need a special integer variable that manages
> this file based on someone setting the FD.  params.c has this sort
> of handling --- see various special integer variables with
> their own get / set functions, e.g. intsecondssetfn shows you
> what happens when someone sets SECONDS. You're going to have
> a function of that sort that instead closes and reopens
> xtrerr.
>
> I guess ZSH_XTRACEFD is the obvious name.
>
> Cheers
> pws
>
>