Bug 531353 - tcsh obeys printexitvalue for back-ticks
tcsh obeys printexitvalue for back-ticks
Status: CLOSED CURRENTRELEASE
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: tcsh (Show other bugs)
5.4
All Linux
urgent Severity medium
: rc
: ---
Assigned To: Jaromír Končický
BaseOS QE - Apps
: Patch, Reopened, ZStream
Depends On:
Blocks: 502912 590060 579251 640252 689381
  Show dependency treegraph
 
Reported: 2009-10-27 16:18 EDT by Jeff Bastian
Modified: 2014-07-01 18:55 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 531957 578817 689381 (view as bug list)
Environment:
Last Closed: 2013-09-23 07:05:54 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch to fix backquoted output (3.14 KB, patch)
2009-10-29 18:29 EDT, Jeff Bastian
no flags Details | Diff

  None (edit)
Description Jeff Bastian 2009-10-27 16:18:03 EDT
Description of problem:
If printexitvalue is set, tcsh returns the exit code as part of the command output, rendering the output unusable.

Version-Release number of selected component (if applicable):
tcsh-6.14-14.el5

How reproducible:
every time

Steps to Reproduce:
1. set printexitvalue
2. set i = `grep "nonono" /etc/csh.cshrc | wc -l`
3. echo $i
  
Actual results:
0 Exit 1

Expected results:
0

Additional info:
RHEL 4 tcsh-6.13-10.el4 behaves correctly
Comment 1 Jeff Bastian 2009-10-27 16:32:29 EDT
This bug was reported a while ago:
  http://mx.gw.com/pipermail/tcsh-bugs/2006-June/000444.html

However, it's still a bug with the latest upstream release, version 6.17.00 from July 2009.
  ftp://ftp.astron.com/pub/tcsh/tcsh-6.17.00.tar.gz
Comment 2 Jeff Bastian 2009-10-27 17:50:57 EDT
Comparing RHEL 4 and RHEL 5 tcsh code, this chunk is new:

--- RHEL-4/tcsh-6.13.00/sh.sem.c        2009-10-27 15:47:02.906665630 -0500   
+++ RHEL-5/tcsh-6.14.00/sh.sem.c        2009-10-27 15:47:07.391667628 -0500
@@ -656,6 +652,13 @@ execute(t, wanttty, pipein, pipeout, do_                   
            func(t, bifunc);                                                    
            if (forked)                                                         
                exitstat();                                                     
+           else {                                                              
+               if (adrof(STRprintexitvalue)) {                                 
+                   int rv = getn(varval(STRstatus));                           
+                   if (rv != 0)                                                
+                       xprintf(CGETS(17, 2, "Exit %d\n"), rv);                 
+               }                                                               
+           }                                                                   
            break;                                                              
        }                                                                       
        if (t->t_dtyp != NODE_PAREN) {


Now, compare that new chunk to the pjwait() function in sh.proc.c:
    /*
     * Don't report on backquoted jobs, cause it will mess up
     * their output.
     */
    if ((reason != 0) && (adrof(STRprintexitvalue)) &&
        (pp->p_flags & PBACKQ) == 0)
        xprintf(CGETS(17, 2, "Exit %d\n"), reason);


The new chunk above does not check pp->p_flags for the PBACKQ flag; I wonder if it should?
Comment 3 Jeff Bastian 2009-10-27 18:18:43 EDT
Ignore comment 2...  I just tested for backquotes in that chunk but that didn't solve the bug.

diff --git a/sh.sem.c b/sh.sem.c
index 0210e54..7fe828e 100644
--- a/sh.sem.c
+++ b/sh.sem.c
@@ -653,7 +653,8 @@ execute(t, wanttty, pipein, pipeout, do_glob)
            if (forked)
                exitstat();
            else {
-               if (adrof(STRprintexitvalue)) {
+               if ((adrof(STRprintexitvalue)) &&
+                    ((t->t_dflg & F_BACKQ) == 0)) {
                    int rv = getn(varval(STRstatus));
                    if (rv != 0)
                        xprintf(CGETS(17, 2, "Exit %d\n"), rv);
Comment 4 Jeff Bastian 2009-10-27 18:33:39 EDT
Well, maybe there is something to the above chunk after all.  I ran tcsh through gdb and set breakpoints on all the print statements that print an 'Exit %d' type string, and it stopped on the xprintf in the new chunk:


$ rpm -q tcsh tcsh-debuginfo
tcsh-6.14-14.el5.x86_64
tcsh-debuginfo-6.14-14.el5.x86_64

$ cat tcsh-gdb-cmds.txt
b sh.proc.c:640
b sh.proc.c:1160
b sh.sem.c:659
run -f

$ gdb -q -x tcsh-gdb-cmds.txt /bin/tcsh
Breakpoint 1 at 0x41e86a: file sh.proc.c, line 640.
Breakpoint 2 at 0x41d18e: file sh.proc.c, line 1160.
Breakpoint 3 at 0x41fb06: file sh.sem.c, line 659.
> set printexitvalue
> set i=`/bin/false`
Detaching after fork from child process 19285.

Breakpoint 3, execute (t=0xb395930, wanttty=19279, pipein=0x0, pipeout=0x0,
    do_glob=1) at sh.sem.c:659
659                             xprintf(CGETS(17, 2, "Exit %d\n"), rv);
(gdb) print rv
$1 = 1
(gdb) c
Continuing.
Exit 1
> exit

Program exited with code 01.
(gdb)
Comment 5 Jeff Bastian 2009-10-28 14:45:13 EDT
I did some more testing and I see how the two xprintf() statements work with each other now: the new chunk of code in sh.sem.c prints the exit value for built-in commands whereas the chunk in sh.proc.c handles normal commands.

I modified each xprintf() slightly so I could tell the difference:
   1. sh.proc.c
   -       xprintf(CGETS(17, 2, "Exit %d\n"), reason);
   +       xprintf(CGETS(17, 2, "BAR %d\n"), reason);

   2. sh.sem.c 
   -                       xprintf(CGETS(17, 2, "Exit %d\n"), rv);
   +                       xprintf(CGETS(17, 2, "FOO %d\n"), rv);

NOTE: You have to unset the LANG environment variable or the CGETS() call will get the string to print from the i18n files.

Running with this patch
   $ ./tcsh -f
   > unsetenv LANG
   > set printexitvalue
   > /bin/false
   BAR 1
   > set i=`/bin/false`
   FOO 1
   > echo $i
   BAR 1
   > exit

Thus, the bug lies in sh.proc.c since $i should be an empty string.
Comment 6 Jeff Bastian 2009-10-28 15:06:04 EDT
It looks like the PBACKQ flag is not getting set in pp->p_flags; I made the xprintf() a little more verbose:

    if ((reason != 0) && (adrof(STRprintexitvalue)) &&
        (pp->p_flags & PBACKQ) == 0)
        xprintf("Exit %d, pp->p_flags & PBACKQ => 0x%x & 0x%x => 0x%x\n",
                reason, pp->p_flags, PBACKQ, (pp->p_flags & PBACKQ));


Running it:

$ ./tcsh -f
> unsetenv LANG
> set printexitvalue
> /bin/false
Exit 1, pp->p_flags & PBACKQ => 0x8 & 0x10000 => 0x0
> set i=`/bin/false`
Exit 1
> echo $i
Exit 1, pp->p_flags & PBACKQ => 0x8 & 0x10000 => 0x0
> exit
Comment 7 Jeff Bastian 2009-10-28 15:58:28 EDT
This is strange: according to gdb, the PBACKQ flag is set, but the xprintf shows that it's NOT set.

> unsetenv LANG
> set printexitvalue
> set i=`/bin/false`
Detaching after fork from child process 14239.

Breakpoint 1, pjwait (pp=0x1b54ffd0) at sh.proc.c:638
638         if ((reason != 0) && (adrof(STRprintexitvalue)) &&
(gdb) p pp->p_flags
$1 = 65544
(gdb) c
Continuing.
Exit 1
> echo $i
Exit 1, pp->p_flags & PBACKQ => 0x8 & 0x10000 => 0x0
> exit


65544 = 0x10008, which means the PBACKQ (0x10000) and PAEXITED (0x8) flags are set, thus it should not call the xprintf(), but it clearly does from the value of $i.

It must be the `/bin/false` child process that's calling xprintf().  So, maybe the PBACKQ flag is not set in the child?  Or maybe printexitvalue should be disabled in back-quoted child processes?
Comment 8 Jeff Bastian 2009-10-29 17:37:15 EDT
gdb was having problems following the child process, so I added a bunch of xprintf("DEBUG...") statements and verified that the PBACKQ flag is getting lost somewhere in the fork-and-exec calls.

I'm working on a patch to make sure the PBACKQ follows the child process.
Comment 9 Jeff Bastian 2009-10-29 18:29:39 EDT
Created attachment 366728 [details]
patch to fix backquoted output

Attached is a patch to set and preserve the F_BACKQ flag (and, in turn, the PBACKQ flag) for backquoted commands.  This fixes the bug in my testing.
Comment 12 Jeff Bastian 2009-10-30 10:02:41 EDT
I sent the patch upstream at
  http://mx.gw.com/pipermail/tcsh-bugs/2009-October/000648.html
Comment 13 Issue Tracker 2009-11-02 04:42:24 EST
Event posted on 2009-11-02 10:42 CET by rdassen

The test packages that Jeff Bastian provided for customer testing (based on
the patch from comment #9) have been successfully tested by the customer:
they confirm the test packages fix the issue and report no new problems.


This event sent from IssueTracker by rdassen 
 issue 358655

Note You need to log in before you can comment on or make changes to this bug.