Re: Feature request: ZSH_XTRACEFD variable

["Timothée Mazzucotelli" <timothee.mazzucotelli@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5452 bytes --]

Hello :)

I managed to write a test for the ZSH_XTRACEFD feature in A04redirect.ztst,
and am planning to add more tests, but maybe you'd have some hints on what
I should test?

Patch in attachment.





On Tue, Aug 13, 2019 at 5:39 PM Peter Stephenson <p.stephenson@samsung.com>
wrote:

> How about this?
>
> I've tried to make it so that if the fd is 2 or less it always uses
> the standard file, if it's greater it always creates or frees one, and
> it only creates a new file when it's requested to switch.
>
> One other fix is that we only need to fclose(xtrerr) which then closes
> the file descriptor.  There's no way of just closing the file and not
> the file descriptor --- we'll need to document that the file descriptor
> is closed if ZSH_XTRACEFD changes again since it's a bit illogical given
> that the user opened the fd in the first place.
>
> pws
>
> diff --git a/Src/exec.c b/Src/exec.c
> index e81053d67..fe702b644 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -5394,7 +5394,7 @@ execshfunc(Shfunc shf, LinkList args)
>      cmdsp = 0;
>      if ((osfc = sfcontext) == SFC_NONE)
>         sfcontext = SFC_DIRECT;
> -    xtrerr = stderr;
> +    xtrerr = xtrace_file;
>
>      doshfunc(shf, args, 0);
>
> diff --git a/Src/init.c b/Src/init.c
> index 445cd3937..c51197079 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;
> +    xtracefdassign();
>
>      /* Make sure the tty is opened read/write. */
>      if (isatty(0)) {
> diff --git a/Src/params.c b/Src/params.c
> index 1499e3a40..2347ee9c3 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,71 @@ setsecondstype(Param pm, int on, int off)
>      return 0;
>  }
>
> +/* Open / assign the XTRACE fd */
> +
> +/**/
> +void xtracefdassign(void)
> +{
> +    int fd = (int)zsh_xtracefd;
> +    switch (fd)
> +    {
> +    case 0:                    /* bizarre, but handle for consistency */
> +       xtrerr = stdin;
> +       break;
> +
> +    case 1:
> +       xtrerr = stdout;
> +       break;
> +
> +    case 2:
> +       xtrerr = stderr;
> +       break;
> +
> +    default:
> +       xtrerr = fdopen(fd, "w");
> +       break;
> +    }
> +    xtrace_file = xtrerr;
> +}
> +
> +/* 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)
> +        fclose(xtrerr);
> +      intvarsetfn(pm, fd);
> +      xtracefdassign();
> +    } 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 > 2)
> +         fclose(xtrerr);  /* Never close standard descriptors */
> +      xtrerr = xtrace_file = stderr;
> +    }
> +}
> +
>  /* Function to get value for special parameter `USERNAME' */
>
>  /**/
> diff --git a/Src/utils.c b/Src/utils.c
> index 46cf7bcf6..2eee27697 100644
> --- a/Src/utils.c
> +++ b/Src/utils.c
> @@ -1760,12 +1760,19 @@ checkmailpath(char **s)
>  /**/
>  FILE *xtrerr = 0;
>
> +/* This records the last file XTRACE was open too.
> + * It's used for restoring XTRACE after a possible redirection.gggg
> + */
> +
> +/**/
> +FILE *xtrace_file;
> +
>  /**/
>  void
>  printprompt4(void)
>  {
>      if (!xtrerr)
> -       xtrerr = stderr;
> +       xtracefdassign();
>      if (prompt4) {
>         int l, t = opts[XTRACE];
>         char *s = dupstring(prompt4);
>

[-- Attachment #1.2: Type: text/html, Size: 7005 bytes --]

[-- Attachment #2: 0001-44752-Implement-ZSH_XTRACEFD-feature.patch --]
[-- Type: text/x-patch, Size: 5280 bytes --]

From 70a069a43fc15eff5163290ecaa79d7c71708460 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timoth=C3=A9e=20Mazzucotelli?= <pawamoy@pm.me>
Date: Sat, 18 Apr 2020 17:22:03 +0200
Subject: [PATCH] 44752: Implement ZSH_XTRACEFD feature

---
 Src/exec.c            |  2 +-
 Src/init.c            |  6 ++--
 Src/params.c          | 72 ++++++++++++++++++++++++++++++++++++++++++-
 Src/utils.c           |  9 +++++-
 Test/A04redirect.ztst | 10 ++++++
 5 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/Src/exec.c b/Src/exec.c
index 2b8e2167f..0e34f5f96 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5421,7 +5421,7 @@ execshfunc(Shfunc shf, LinkList args)
     cmdsp = 0;
     if ((osfc = sfcontext) == SFC_NONE)
 	sfcontext = SFC_DIRECT;
-    xtrerr = stderr;
+    xtrerr = xtrace_file;
 
     doshfunc(shf, args, 0);
 
diff --git a/Src/init.c b/Src/init.c
index 3d6c94d04..89c50b17e 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -616,8 +616,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;
+    xtracefdassign();
 
     /* Make sure the tty is opened read/write. */
     if (isatty(0)) {
diff --git a/Src/params.c b/Src/params.c
index 863b32600..569c21304 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -106,7 +106,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    */
 /**/
@@ -268,6 +269,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
@@ -357,6 +361,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}
@@ -4399,6 +4404,71 @@ setsecondstype(Param pm, int on, int off)
     return 0;
 }
 
+/* Open / assign the XTRACE fd */
+
+/**/
+void xtracefdassign(void)
+{
+    int fd = (int)zsh_xtracefd;
+    switch (fd)
+    {
+    case 0:                    /* bizarre, but handle for consistency */
+       xtrerr = stdin;
+       break;
+
+    case 1:
+       xtrerr = stdout;
+       break;
+
+    case 2:
+       xtrerr = stderr;
+       break;
+
+    default:
+       xtrerr = fdopen(fd, "w");
+       break;
+    }
+    xtrace_file = xtrerr;
+}
+
+/* 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)
+        fclose(xtrerr);
+      intvarsetfn(pm, fd);
+      xtracefdassign();
+    } 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 > 2)
+         fclose(xtrerr);  /* Never close standard descriptors */
+      xtrerr = xtrace_file = stderr;
+    }
+}
+
 /* Function to get value for special parameter `USERNAME' */
 
 /**/
diff --git a/Src/utils.c b/Src/utils.c
index 69885fed3..a817c9aa8 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1760,12 +1760,19 @@ checkmailpath(char **s)
 /**/
 FILE *xtrerr = 0;
 
+/* This records the last file XTRACE was open too.
+ * It's used for restoring XTRACE after a possible redirection.
+ */
+
+/**/
+FILE *xtrace_file;
+
 /**/
 void
 printprompt4(void)
 {
     if (!xtrerr)
-	xtrerr = stderr;
+	xtracefdassign();
     if (prompt4) {
 	int l, t = opts[XTRACE];
 	char *s = dupstring(prompt4);
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index d60519064..fd06854e1 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -708,3 +708,13 @@
   cat <&$testfd
 0:Regression test for here document with fd declarator
 >  This is, in some sense, a here document.
+
+  rm -f redir
+  set -x
+  ZSH_XTRACEFD=4 print 'This is ZSH_XTRACEFD redir' 4>redir
+  set +x
+  cat redir
+0:Redirect xtrace output to ZSH_XTRACEFD file descriptor
+>This is ZSH_XTRACEFD redir
+>+(eval):3> print 'This is ZSH_XTRACEFD redir'
+?+(eval):3> ZSH_XTRACEFD=4 +(eval):4> set +x
-- 
2.26.0