Subject: Re: bin/33956: -current /bin/sh possible regression
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, njoly@pasteur.fr>
From: David Holland <dholland+netbsd@eecs.harvard.edu>
List: netbsd-bugs
Date: 07/10/2006 22:10:04
The following reply was made to PR bin/33956; it has been noted by GNATS.

From: dholland+netbsd@eecs.harvard.edu (David Holland)
To: current-users@netbsd.org, njoly@pasteur.fr
Cc: gnats-bugs@netbsd.org
Subject: Re: bin/33956: -current /bin/sh possible regression
Date: Mon, 10 Jul 2006 18:05:32 -0400 (EDT)

 Nicolas Joly <njoly@pasteur.fr> wrote:
  > I just noticed that -current /bin/sh does not give the expected
  > results (note the missing dots ...) with the following piece of code:
  >
  > [snip]
  >
  > I'm pretty sure it worked previously, but i can't remember when.
 
 Whether or not it's a regression, it's definitely a bug.
 
 Shorter test case:
 
    sh -c 'foo() { echo "Testing ${@} fnord"; }; foo a'
    sh -c 'foo() { echo "Testing ${@} fnord"; }; foo a b'
 
 produces
 
    Testing a
    Testing a b  fnord
 
 I thought at first the problem was this code
 
 	apply_ifs = ((varflags & VSQUOTE) == 0 ||
 		(*var == '@' && shellparam.nparam != 1));
 
 at line 685 of expand.c (version 1.74), which doesn't set apply_ifs if
 there's only one argument. If you don't apply ifs expansion, an
 embedded null gets left behind and the trailing material thus gets
 lost.
 
 However, that turns out to be a red herring. The real problem is that
 extra space after the 'b'. Where's it coming from?
 
    sh -c 'foo() { printargv "Testing ${@} fnord"; }; foo a b'
 
 (where printargv is the obvious thing) produces
 
    argv[0]: -printargv-
    argv[1]: -Testing a-
    argv[2]: -b-
    argv[3]: - fnord-
 
 whereas with ksh, bash, and zsh you get
 
 argv[0]: -printargv-
 argv[1]: -Testing a-
 argv[2]: -b fnord-
 
 It turns out that there's an extra embedded null (these are used for
 dividing arguments within a quoted string) left at the end of the
 expansion of @, which causes b to be its own argument, so the extra
 space is inserted by echo.
 
 Patch:
 
 Index: expand.c
 ===================================================================
 RCS file: /cvsroot/src/bin/sh/expand.c,v
 retrieving revision 1.74
 diff -u -r1.74 expand.c
 --- expand.c	20 May 2006 13:57:27 -0000	1.74
 +++ expand.c	10 Jul 2006 21:57:47 -0000
 @@ -873,7 +873,8 @@
  			for (ap = shellparam.p ; (p = *ap++) != NULL ; ) {
  				STRTODEST(p);
  				/* Nul forces a parameter split inside "" */
 -				STPUTC('\0', expdest);
 +				if (*ap) 
 +					STPUTC('\0', expdest);
  			}
  			break;
  		}
 
 Cheers. :-)
 
 -- 
    - David A. Holland / dholland@eecs.harvard.edu