zsh-workers
 help / color / mirror / code / Atom feed
From: "Timothée Mazzucotelli" <timothee.mazzucotelli@gmail.com>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: Peter Stephenson <p.w.stephenson@ntlworld.com>,
	 "zsh-workers@zsh.org" <zsh-workers@zsh.org>
Subject: Re: Feature request: ZSH_XTRACEFD variable
Date: Thu, 3 Sep 2020 15:51:36 +0200	[thread overview]
Message-ID: <CAD8ZDTrVFa5Yt3ReEmxFUvZ_2U4aVgrHiyQLGxje8SgFhGmg+w@mail.gmail.com> (raw)
In-Reply-To: <CAH+w=7YpEEy-H8tXt6toaC3gNQ95SAgjGJ=_n5_+XwXCchC3+Q@mail.gmail.com>


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

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@brasslantern.com>
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@ntlworld.com> 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.
>

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

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

From b9b37333fcf02a463f6f742976b37b45ab08742d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timoth=C3=A9e=20Mazzucotelli?= <pawamoy@pm.me>
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


  reply	other threads:[~2020-09-03 13:52 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17 15:08 Timothée Mazzucotelli
2019-05-18  7:55 ` Stephane Chazelas
2019-05-20 10:34   ` Stephane Chazelas
2019-07-21 15:08     ` Timothée Mazzucotelli
2019-07-21 15:22       ` Peter Stephenson
2019-07-31 19:40         ` Timothée Mazzucotelli
2019-07-31 19:45           ` Timothée Mazzucotelli
2019-07-31 21:12             ` Sebastian Gniazdowski
2019-08-13  9:14           ` Peter Stephenson
2019-08-13 15:38           ` Peter Stephenson
2019-09-10 22:19             ` Timothée Mazzucotelli
2019-09-11  8:45               ` Peter Stephenson
2020-04-19 10:30             ` Timothée Mazzucotelli
2020-04-20 14:09               ` Peter Stephenson
2020-05-02 18:02                 ` Timothée Mazzucotelli
2020-05-02 21:15                   ` Bart Schaefer
2020-05-03  0:06                     ` Daniel Shahaf
2020-05-03  4:43                       ` Roman Perepelitsa
2020-05-03 16:21                         ` Daniel Shahaf
2020-05-03 19:54                         ` Peter Stephenson
2020-05-03 21:06                           ` Daniel Shahaf
2020-05-04  8:35                             ` Peter Stephenson
2020-05-05  0:03                               ` Daniel Shahaf
2020-05-05 16:36                                 ` Timothée Mazzucotelli
2020-05-05 16:47                                   ` Peter Stephenson
2020-05-05 22:19                                     ` Bart Schaefer
2020-09-03 13:51                                       ` Timothée Mazzucotelli [this message]
2020-09-04 19:48                                         ` Daniel Shahaf
2020-09-04 19:53                                           ` Bart Schaefer
2020-09-05  9:24                                             ` Timothée Mazzucotelli
2020-05-05 22:11                                   ` Bart Schaefer
2020-05-03  6:01                       ` Stephane Chazelas
2020-05-03  7:07                         ` Stephane Chazelas
2020-05-03 17:30                   ` Peter Stephenson
2020-05-03 21:23                     ` Daniel Shahaf
2020-05-04  8:26                       ` Peter Stephenson
2020-05-05 20:50               ` Daniel Shahaf

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=CAD8ZDTrVFa5Yt3ReEmxFUvZ_2U4aVgrHiyQLGxje8SgFhGmg+w@mail.gmail.com \
    --to=timothee.mazzucotelli@gmail.com \
    --cc=p.w.stephenson@ntlworld.com \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

	https://git.vuxu.org/mirror/zsh/

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