Subject: Re: bin/33956: -current /bin/sh possible regression
To: current-users@netbsd.org, <j+nbsd@2006.salmi.ch>
From: David Holland <dholland+netbsd@eecs.harvard.edu>
List: current-users
Date: 07/11/2006 14:33:52
Jukka Salmi <j+nbsd@2006.salmi.ch> wrote:
 > Hmm, this reverts what has been recently [1]changed to fix PR
 > [2]bin/33472...
 > 
 > Cheers, Jukka
 > 
 > [1] http://cvsweb.netbsd.org/bsdweb.cgi/src/bin/sh/expand.c.diff?r1=1.73&r2=1.74
 > [2] http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=33472

Hrm, good point.

*investigates*

This additional patch fixes that bug:

--- expand.c~	2006-07-10 17:56:18.000000000 -0400
+++ expand.c	2006-07-11 14:15:31.000000000 -0400
@@ -1035,7 +1035,7 @@
 	 * Some recent clarification of the Posix spec say that it
 	 * should only generate one....
 	 */
-	if (*start) {
+	if (*start || inquotes) {
 		sp = (struct strlist *)stalloc(sizeof *sp);
 		sp->text = start;
 		*arglist->lastp = sp;


and here's a patch for the regression suite (which doesn't all pass,
but my hacking hasn't changed that)

--- regress/bin/sh/expand.sh~	2006-05-11 20:05:59.000000000 -0400
+++ regress/bin/sh/expand.sh	2006-07-11 14:22:21.000000000 -0400
@@ -27,3 +27,11 @@
 set -- "" ""				# This code triggered the bug.
 got=`echo "$@" | sed 's,$,EOL,'`
 assert_equals "2" " EOL" "$got"
+
+# The original fix wasn't quite right, and this triggers that bug.
+foo() { echo "Testing ${@} fnord"; }
+got=`foo a`
+assert_equals "3" "Testing a fnord" "$got"
+got=`foo a b`
+assert_equals "3" "Testing a b fnord" "$got"
+

--- regress/bin/sh/Makefile~	2006-06-16 10:43:17.000000000 -0400
+++ regress/bin/sh/Makefile	2006-07-11 14:28:46.000000000 -0400
@@ -10,6 +10,6 @@
 	sh ${.CURDIR}/fsplit.sh
 	sh ${.CURDIR}/here.sh
 	sh ${.CURDIR}/set_e.sh
-	#sh ${.CURDIR}/expand.sh	# First needs to be fixed.
+	sh ${.CURDIR}/expand.sh
 
 .include <bsd.prog.mk>


-- 
   - David A. Holland / dholland@eecs.harvard.edu