Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: Feature request: ZSH_XTRACEFD variable



Hello, I'm coming back to the ZSH_XTRACEFD feature again :)

About closing file descriptors or not: I don't understand what it means to "leak a FILE".
I'm not sure to understand the past comments either: we only use fdopen when the value of ZSH_XTRACEFD is > 2.
For 0, 1 and 2, we re-use the existing file descriptors stdin, stdout and stderr.

Anyway, I added the patch in an attachment. Also, here's the link to the commit on my fork:
https://github.com/pawamoy/zsh/commit/b9b37333fcf02a463f6f742976b37b45ab08742d

In this patch, I never close any file descriptor.

There's one last thing that looks weird to me:
single instructions like ZSH_XTRACEFD=5 are not properly logged in the xtrace output.
It seems they are truncated up to the end of the variable assignment:
- with a=0 ZSH_XTRACEFD=5, nothing appear in the output either
- with ZXH_XTRACEFD=5 a=0, only a=0 appears in the output (but no +(eval):18 prefix or similar)

Any idea about this?

Cheers,
Timothée

On Wed, May 6, 2020 at 12:20 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
(Peter, for some reason Gmail is classifying all email from
ntlworld.com as spam, with the notation that it "can't guarantee that
this message came from ntlworld.com")

On Tue, May 5, 2020 at 9:48 AM Peter Stephenson
<p.w.stephenson@xxxxxxxxxxxx> wrote:
>
>
> The problem is if we fopen() the file descriptor to use stdio as output, we can either
> leak the entire FILE, not opened by the user, or we can close the entire FILE.

In that case we should be doing the fopen() on a dup() of the
descriptor, and fclose()ing the FILE.

If it is important that fileno(xtrerr) == $ZSH_XTRACEFD, then we should
1) dup() the descriptor to save a copy
2) fopen() the original
3) after fclose(), dup2() the copy back to the original
4) close() the copy

However, I'm not sure it's necessary to be that convoluted.
From b9b37333fcf02a463f6f742976b37b45ab08742d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timoth=C3=A9e=20Mazzucotelli?= <pawamoy@xxxxx>
Date: Thu, 3 Sep 2020 11:27:16 +0200
Subject: [PATCH] 44752: Implement ZSH_XTRACEFD feature

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

diff --git a/Src/exec.c b/Src/exec.c
index ecad923de..360dce0ee 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5453,7 +5453,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 122f5da7d..87208fca3 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,65 @@ 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)
+{
+    /* Check that the given file descriptor is valid */
+    if (fcntl(fd, F_GETFD) != -1 || errno != EBADF) {
+      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 5151b89a8..efe28c9c6 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 993138e7d..1e34d4961 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -722,3 +722,70 @@
 >Works
 >Works
 ?(eval):6: file exists: foo
+
+  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
+
+  rm -f redir
+  A() {
+    local ZSH_XTRACEFD=5
+    B
+    print 'Function A to file descriptor 5'
+    unset ZSH_XTRACEFD
+    print 'Function A to file descriptor 2'
+  }
+  B() {
+    local ZSH_XTRACEFD=6
+    print 'Function B to file descriptor 6'
+  }
+  exec 4>redir4 5>redir5 6>redir6
+  ZSH_XTRACEFD=4
+  set -x
+  print 'Main to file descriptor 4'
+  A
+  print 'Main to file descriptor 4 again\n'
+  a=0 ZSH_XTRACEFD=5  # appears as blank line in redir5
+  ZSH_XTRACEFD=6  # appears as blank line in redir6
+  unset ZSH_XTRACEFD
+  set +x
+  print "end of file redir4" >> redir4
+  cat redir4
+  print
+  print "end of file redir5" >> redir5
+  cat redir5
+  print
+  print "end of file redir6" >> redir6
+  cat redir6
+0:Scoped ZSH_XTRACEFD correctly set and restored
+>Main to file descriptor 4
+>Function B to file descriptor 6
+>Function A to file descriptor 5
+>Function A to file descriptor 2
+>Main to file descriptor 4 again
+>
+>+(eval):16> print 'Main to file descriptor 4'
+>+(eval):17> A
+>+A:1> local ZSH_XTRACEFD=5
+>+(eval):18> print 'Main to file descriptor 4 again\n'
+>end of file redir4
+>
+>+A:2> B
+>+B:1> local ZSH_XTRACEFD=6
+>+A:3> print 'Function A to file descriptor 5'
+>+A:4> unset ZSH_XTRACEFD
+>
+>end of file redir5
+>
+>+B:2> print 'Function B to file descriptor 6'
+>
+>+(eval):21> unset ZSH_XTRACEFD
+>end of file redir6
+?+A:5> print 'Function A to file descriptor 2'
+?+(eval):22> set +x
-- 
2.28.0



Messages sorted by: Reverse Date, Date, Thread, Author