0000110: [PATCH] Incorrect $status value of pipelined commands - bugs.gw.com

bugs.gw.com

Mantis Bug Tracker

View Issue Details [ Jump to Notes ]

[ Issue History ] [ Print ]

ID

Project

Category

View Status

Date Submitted

Last Update

0000110

tcsh

general

public

2010-12-02 11:37

2011-04-14 18:24

Reporter

Vojtech Vitek (V-Teq)

Assigned To

Christos Zoulas

Priority

normal

Severity

major

Reproducibility

always

Status

closed

Resolution

fixed

Platform

OS

OS Version

Product Version

6.17.00

Target Version

Fixed in Version

HEAD

Summary

0000110: [PATCH] Incorrect $status value of pipelined commands

Description

The tcsh documentation states that the $status (or $?) variables hold the
return value of the last command. Therefore in case of pipelines it should be
the last command of the pipeline. This doesn't work properly.

There are two examples:

1)
$ cat /notfound | wc -l
cat: /notfound: No such file or directory
0
$ echo $?
1 <------- should be 0 (the last command - wc - was successful)

2)
$ echo `cat /notfound`
cat: /notfound: No such file or directory

$ echo $?
1 <------- should be 0 (echo was successful)

This bug was originally reported in https://bugzilla.redhat.com/show_bug.cgi?id=638955 [^]

I'm attaching a patch by Tomas Smetana <tsmetana AT redhat.com>.

Tags

No tags attached.

tcsh-6.14.00-pipeexit.patch [^] (556 bytes) 2010-12-02 11:37 [Show Content] [Hide Content]

diff -up tcsh-6.14.00/sh.proc.c.pipeexec tcsh-6.14.00/sh.proc.c
--- tcsh-6.14.00/sh.proc.c.pipeexec	2010-10-15 10:49:20.647126202 +0200
+++ tcsh-6.14.00/sh.proc.c	2010-10-15 13:25:51.203378381 +0200
@@ -627,6 +627,10 @@ pjwait(pp)
     reason = 0;
     fp = pp;
     do {
+	/* In case of pipelines only the result of the last
+	 * command should be taken in account */
+	if ((fp->p_flags & PPOU) || (fp->p_flags & PBACKQ))
+	    continue;
 	if (fp->p_reason)
 	    reason = fp->p_flags & (PSIGNALED | PINTERRUPTED) ?
 		fp->p_reason | META : fp->p_reason;

tcsh-6.17.00-pipe-exit-status.patch [^] (1,580 bytes) 2011-04-14 09:34 [Show Content] [Hide Content]

diff -up tcsh-6.14.00/sh.proc.c.pipeexec tcsh-6.14.00/sh.proc.c
--- tcsh-6.14.00/sh.proc.c.pipeexec	2010-10-15 10:49:20.647126202 +0200
+++ tcsh-6.14.00/sh.proc.c	2010-10-15 13:25:51.203378381 +0200
@@ -627,6 +627,11 @@ pjwait(pp)
     reason = 0;
     fp = pp;
     do {
+	/* In case of pipelines only the result of the last
+	 * command should be taken in account */
+	if (!(fp->p_flags & PBRACE)
+		&& ((fp->p_flags & PPOU) || (fp->p_flags & PBACKQ)))
+	    continue;
 	if (fp->p_reason)
 	    reason = fp->p_flags & (PSIGNALED | PINTERRUPTED) ?
 		fp->p_reason | META : fp->p_reason;
diff -up tcsh-6.14.00/sh.proc.c.retval tcsh-6.14.00/sh.proc.c
--- tcsh-6.14.00/sh.proc.c.retval	2011-02-03 14:56:12.000000000 +0100
+++ tcsh-6.14.00/sh.proc.c	2011-02-03 14:57:02.000000000 +0100
@@ -800,6 +805,8 @@ palloc(pid, t)
 	pp->p_flags |= PBACKQ;
     if (t->t_dflg & F_HUP)
 	pp->p_flags |= PHUP;
+    if (t->t_dcom && t->t_dcom[0] && (*t->t_dcom[0] == '{'))
+	pp->p_flags |= PBRACE;
     if (cmdmax == 0)
 	morecommand(CMD_INIT);
     cmdp = cmdstr;
diff -up tcsh-6.14.00/sh.proc.h.retval tcsh-6.14.00/sh.proc.h
--- tcsh-6.14.00/sh.proc.h.retval	2004-11-23 03:10:49.000000000 +0100
+++ tcsh-6.14.00/sh.proc.h	2011-02-03 14:56:12.000000000 +0100
@@ -102,6 +102,7 @@ struct process {
 #define	PNEEDNOTE	(1<<15)	/* notify as soon as practical */
 #define PBACKQ		(1<<16)	/* Process is `` evaluation */
 #define PHUP		(1<<17)	/* Process is marked for SIGHUP on exit */
+#define PBRACE		(1<<18)	/* Process is {} evaluation */
 
 /* defines for arguments to pprint */
 #define	NUMBER		01

tcsh-6.17.00-testsuite.patch [^] (1,261 bytes) 2011-04-14 09:38 [Show Content] [Hide Content]

diff --git a/tests/lexical.at b/tests/lexical.at
index 70a2575..02534fe 100644
--- a/tests/lexical.at
+++ b/tests/lexical.at
@@ -57,7 +57,7 @@ echo "&|;<>()&||<<>>space tab	end"
 set verbose
 echo `&|;<>()&||<<>>space tab	end`
 ]])
-AT_CHECK([tcsh -f nosplit.csh], 1,
+AT_CHECK([tcsh -f nosplit.csh], 0,
 [&|;<>()&||<<>>space tab	end
 &|;<>()&||<<>>space tab	end
 &|;<>()&||<<>>space tab	end
@@ -132,7 +132,7 @@ AT_CHECK([tcsh -f -c "echo !OK"], 1, ,
 AT_CHECK([tcsh -f -c 'echo "$OK"'], 1, , [OK: Undefined variable.
 ])
 
-AT_CHECK([tcsh -f -c 'echo "`OK`"'], 1, [
+AT_CHECK([tcsh -f -c 'echo "`OK`"'], 0, [
 ],
 [OK: Command not found.
 ])
diff --git a/tests/subst.at b/tests/subst.at
index 71feb97..a59171b 100644
--- a/tests/subst.at
+++ b/tests/subst.at
@@ -14,7 +14,7 @@ set csubstnonl
 echo `echo 1; \\
   echo 2`
 ]])
-AT_CHECK([tcsh -f backq.csh], 1,
+AT_CHECK([tcsh -f backq.csh], 0,
 [4
 2
 1 2
diff --git a/tests/syntax.at b/tests/syntax.at
index 2a2abd0..4908ba3 100644
--- a/tests/syntax.at
+++ b/tests/syntax.at
@@ -156,7 +156,7 @@ AT_CHECK([cat output], ,
 [OK
 ])
 
-AT_CHECK([tcsh -f -c '(echo $this_does_not_exist) |& cat'], 1,
+AT_CHECK([tcsh -f -c '(echo $this_does_not_exist) |& cat'], 0,
 [this_does_not_exist: Undefined variable.
 ])

Relationships

Notes
(0000157) Christos Zoulas (manager) 2011-01-17 19:20	revision 3.115 date: 2011/01/17 16:18:26; author: christos; state: Exp; lines: +6 -2 PR/110: exit status should be the status of the last command in a pipeline so ignore previous statuses from the pipeline stages.

(0000166) Christos Zoulas (manager) 2011-01-19 02:36	The patch is wrong; it breaks the unit-tests and has been reverted.

(0000172) Vojtech Vitek (V-Teq) (reporter) 2011-04-14 09:33 edited on: 2011-04-14 09:39	Here are another two patches that should fix 1) the reported issue 2) "The patch is wrong; it breaks the unit-tests and has been reverted." The first patch =============== Fixes 'exit { false }' regression of the original patch.

(0000173) Vojtech Vitek (V-Teq) (reporter) 2011-04-14 09:37	The second patch ================ Fixes the testsuite which is actually wrong.

(0000174) Christos Zoulas (manager) 2011-04-14 18:24	applied to head, and added $anyerror to reproduce the old behavior.

Issue History
Date Modified	Username	Field	Change
2010-12-02 11:37	Vojtech Vitek (V-Teq)	New Issue
2010-12-02 11:37	Vojtech Vitek (V-Teq)	File Added: tcsh-6.14.00-pipeexit.patch
2011-01-17 19:20	Christos Zoulas	Status	new => resolved
2011-01-17 19:20	Christos Zoulas	Fixed in Version	=> HEAD
2011-01-17 19:20	Christos Zoulas	Resolution	open => fixed
2011-01-17 19:20	Christos Zoulas	Assigned To	=> Christos Zoulas
2011-01-17 19:20	Christos Zoulas	Note Added: 0000157
2011-01-19 02:35	Christos Zoulas	Assigned To	Christos Zoulas =>
2011-01-19 02:36	Christos Zoulas	Note Added: 0000166
2011-01-19 02:36	Christos Zoulas	Assigned To	=> Christos Zoulas
2011-01-19 02:36	Christos Zoulas	Status	resolved => acknowledged
2011-04-14 09:33	Vojtech Vitek (V-Teq)	Note Added: 0000172
2011-04-14 09:34	Vojtech Vitek (V-Teq)	File Added: tcsh-6.17.00-pipe-exit-status.patch
2011-04-14 09:37	Vojtech Vitek (V-Teq)	Note Added: 0000173
2011-04-14 09:38	Vojtech Vitek (V-Teq)	File Added: tcsh-6.17.00-testsuite.patch
2011-04-14 09:39	Vojtech Vitek (V-Teq)	Note Edited: 0000172
2011-04-14 18:24	Christos Zoulas	Status	acknowledged => closed
2011-04-14 18:24	Christos Zoulas	Note Added: 0000174

Copyright © 2000 - 2012 MantisBT Group