PATCH: Re: redirection and xtrace output

["Bart Schaefer" <schaefer@candle.brasslantern.com>]

On Feb 19,  7:19pm, Bart Schaefer wrote:
} Subject: PATCH: redirection and xtrace output
}
} This patch introduces a global FILE *xtrerr which is used for (hopefully)
} all xtrace output.
} 
} There is still one problem with this:  The xtrace output of ( ... ) and
} { ... } constructs is NOT redirected with stderr, which it should be.

There was actually one other problem:  When tracing an external command,
the call to closem() was killing fileno(xtrerr) before it was flushed,
so some trace output didn't appear.

The following fixes all these problems, and also catches cases I missed
where execcmd() could return without closing/restoring xtrerr.  It took
me a ridiculously long time to figure out the right test to use for the
( ... ) and { ... } cases, but it turned out to be quite simple.  In the
process I created a test script; I arbitrarily numbered it after the cd
test, becaue it didn't seem to be in the same category as the others.

Index: Src/exec.c
===================================================================
@@ -129,7 +129,7 @@
 /**/
 mod_export Funcstack funcstack;
 
-#define execerr() if (!forked) { lastval = 1; return; } else _exit(1)
+#define execerr() if (!forked) { lastval = 1; goto done; } else _exit(1)
 
 static LinkList args;
 static int doneps4;
@@ -473,6 +473,7 @@
     }
 
     argv = makecline(args);
+    closem(3);
     child_unblock();
     if ((int) strlen(arg0) >= PATH_MAX) {
 	zerr("command too long: %s", arg0, 0);
@@ -1937,9 +1938,12 @@
 
     /* Make a copy of stderr for xtrace output before redirecting */
     fflush(xtrerr);
-    if (xtrerr == stderr &&
-	!(xtrerr = fdopen(movefd(dup(fileno(stderr))), "w")))
-	xtrerr = stderr;
+    if (xtrerr == stderr && (type < WC_SUBSH || type == WC_TIMED)) {
+	if (!(xtrerr = fdopen(movefd(dup(fileno(stderr))), "w")))
+	    xtrerr = stderr;
+	else
+	    fdtable[fileno(xtrerr)] = 3;
+    }
 
     /* Add pipeline input/output to mnodes */
     if (input)
Index: Test/.distfiles
===================================================================
@@ -3,5 +3,5 @@
     ztst.zsh
     01grammar.ztst 02alias.ztst 03quoting.ztst 04redirect.ztst
     05command.ztst 06arith.ztst 07cond.ztst 08traps.ztst 09funcdef.ztst
-    10prompt.ztst 50cd.ztst
+    10prompt.ztst 50cd.ztst 51xtrace.ztst
 '

--- /dev/null	Tue May  5 13:32:27 1998
+++ Test/51xtrace.ztst	Sat Feb 19 16:53:07 2000
@@ -0,0 +1,60 @@
+# Test that xtrace output is correctly generated
+
+%prep
+  mkdir xtrace.tmp && cd xtrace.tmp
+
+%test
+
+  set -x
+  print 'Tracing: builtin'
+  print 'Tracing: builtin 2>file' 2>xtrace.err
+  cat <<<'Tracing: external'
+  cat <<<'Tracing: external 2>file' 2>>xtrace.err
+  ( print 'Tracing: ( builtin )' )
+  ( print 'Tracing: ( builtin ) 2>file' ) 2>>xtrace.err
+  ( cat <<<'Tracing: ( external )' )
+  ( cat <<<'Tracing: ( external ) 2>file' ) 2>>xtrace.err
+  { print 'Tracing: { builtin }' }
+  { print 'Tracing: { builtin } 2>file' } 2>>xtrace.err
+  { cat <<<'Tracing: { external }' }
+  { cat <<<'Tracing: { external } 2>file' } 2>>xtrace.err
+  repeat 1 do print 'Tracing: do builtin done'; done
+  repeat 1 do print 'Tracing: do builtin done 2>file'; done 2>>xtrace.err
+  repeat 1 do cat <<<'Tracing: do external done'; done
+  repeat 1 do cat <<<'Tracing: do external done'; done 2>>xtrace.err
+  set +x
+  cat xtrace.err
+0:xtrace with and without redirection
+>Tracing: builtin
+>Tracing: builtin 2>file
+>Tracing: external
+>Tracing: external 2>file
+>Tracing: ( builtin )
+>Tracing: ( builtin ) 2>file
+>Tracing: ( external )
+>Tracing: ( external ) 2>file
+>Tracing: { builtin }
+>Tracing: { builtin } 2>file
+>Tracing: { external }
+>Tracing: { external } 2>file
+>Tracing: do builtin done
+>Tracing: do builtin done 2>file
+>Tracing: do external done
+>Tracing: do external done
+>+ZTST_execchunk:2> print Tracing: ( builtin ) 2>file
+>+ZTST_execchunk:2> cat
+>+ZTST_execchunk:2> print Tracing: { builtin } 2>file
+>+ZTST_execchunk:2> cat
+>+ZTST_execchunk:2> print Tracing: do builtin done 2>file
+>+ZTST_execchunk:2> cat
+?+ZTST_execchunk:2> print Tracing: builtin
+?+ZTST_execchunk:2> print Tracing: builtin 2>file
+?+ZTST_execchunk:2> cat
+?+ZTST_execchunk:2> cat
+?+ZTST_execchunk:2> print Tracing: ( builtin )
+?+ZTST_execchunk:2> cat
+?+ZTST_execchunk:2> print Tracing: { builtin }
+?+ZTST_execchunk:2> cat
+?+ZTST_execchunk:2> print Tracing: do builtin done
+?+ZTST_execchunk:2> cat
+?+ZTST_execchunk:2> set +x

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com