From owner-freebsd-bugs Thu Apr 20 3:19:52 2000 Delivered-To: freebsd-bugs@freebsd.org Received: from knight.cons.org (knight.cons.org [194.233.237.86]) by hub.freebsd.org (Postfix) with ESMTP id 74FDA37BC6B; Thu, 20 Apr 2000 03:19:46 -0700 (PDT) (envelope-from cracauer@knight.cons.org) Received: (from cracauer@localhost) by knight.cons.org (8.9.3/8.9.3) id MAA14860; Thu, 20 Apr 2000 12:19:43 +0200 (CEST) Date: Thu, 20 Apr 2000 12:19:43 +0200 From: Martin Cracauer To: Anatoly Vorobey Cc: freebsd-bugs@FreeBSD.ORG, bde@FreeBSD.ORG, tegge@FreeBSD.ORG Subject: Re: bin/6577: /bin/sh environment variables not set in simple commands Message-ID: <20000420121942.A14798@cons.org> References: <200004200630.XAA07459@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailer: Mutt 1.0.1i In-Reply-To: <200004200630.XAA07459@freefall.freebsd.org>; from mellon@pobox.com on Wed, Apr 19, 2000 at 11:30:02PM -0700 Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org Thanks for the proposed fix. Looks good to me. Bruce, Tor, I cc'ed you in case you want to have a look at it. If you don't object, I will commit the fix. In <200004200630.XAA07459@freefall.freebsd.org>, Anatoly Vorobey wrote: > The following reply was made to PR bin/6577; it has been noted by GNATS. > > From: Anatoly Vorobey > To: freebsd-gnats-submit@freebsd.org > Cc: > Subject: Re: bin/6577: /bin/sh environment variables not set in simple commands > Date: Thu, 20 Apr 2000 09:27:28 +0000 > > >Gambit-C version 3 depends upon > >FOO=bar eval echo \$FOO > >outputting "bar" to build. It outputs nothing. This is contrary to > >the documented behaviour (for Simple Commands) and to observed > >behaviour on AiX, Digital Unix and on bash. > > I have a fix for this. It's a "I know what I'm doing" fix rather than > "this seems to work" fix; so here's a bit of explanation > which hopefully will make it easier for someone to review and commit ;) > > Basically, our sh(1) keeps internal environment, defined by FOO=bar pairs > before the command, pointed to by variable cmdenviron defined in eval.c . > It merges it with the global environment only when executing external > commands. The builtin commands, on the other hands, are expected to use > bltinlookup() which will look both into cmdenviron and the global > environment, instead of vanilla lookupvar() which only looks into the latter. > > The simple builtin commands which are affected by variables all do that > (a good example is 'cd' checking $HOME). However, "eval" itself is > implemented as a builtin command, which will call evalstr -- the "real" > eval used also by the command loop. When this "real" eval which parses > and executes all commands needs to look into a variable, it happens in > codepath evalstr->evalcommand->expandarg->argstr->evalvar->lookupvar. > Because of this last lookupvar() call, the builtin eval effectively > ignores its internal environment, even though it's there in "cmdenviron" > while it executes. I have changed that call from lookupvar() to bltinlookup(). > Since the whole codepath basically only ever comes from inside eval > processing, it's okay as we do want eval to always remember about internal > environment just in case it's being executed as a builtin. > > An additional gotcha is that earlier evalcommand() never bothered to > clear cmdenviron, and it could hold internal environment of some builtin > command long after it's done executing, which was okay since all builtin > commands were executed from evalcommand() and it would reset cmdenviron > before launching one. This is no more acceptable since now evalcommand() > will consult cmdenviron just to parse its arguments, and when this > evalcommand() does *not* execute as a result of a builtin call, cmdenviron > may contain remains of a previous builtin's environment. Thus I clear > cmdenviron upon a builtin's exit now. > > Testing: well, I've played with it. I've tested that things like > FOO=bar eval echo \$FOO now work correctly. I've built a few worlds. > I've run a few configures. > > Index: eval.c > =================================================================== > RCS file: /freebsd/cvs/src/bin/sh/eval.c,v > retrieving revision 1.27 > diff -u -r1.27 eval.c > --- eval.c 1999/12/20 13:42:59 1.27 > +++ eval.c 2000/04/20 08:45:47 > @@ -850,6 +850,7 @@ > exitstatus = (*builtinfunc[cmdentry.u.index])(argc, argv); > flushall(); > cmddone: > + cmdenviron = NULL; > out1 = &output; > out2 = &errout; > freestdout(); > Index: expand.c > =================================================================== > RCS file: /freebsd/cvs/src/bin/sh/expand.c,v > retrieving revision 1.31 > diff -u -r1.31 expand.c > --- expand.c 1999/12/15 11:46:32 1.31 > +++ expand.c 2000/04/20 08:47:23 > @@ -667,7 +667,7 @@ > set = varisset(var, varflags & VSNUL); > val = NULL; > } else { > - val = lookupvar(var); > + val = bltinlookup(var, 1); > if (val == NULL || ((varflags & VSNUL) && val[0] == '\0')) { > val = NULL; > set = 0; > > -- > Anatoly Vorobey, > mellon@pobox.com http://pobox.com/~mellon/ > "Angels can fly because they take themselves lightly" - G.K.Chesterton > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-bugs" in the body of the message -- %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% Martin Cracauer http://www.cons.org/cracauer/ Tel.: (private) +4940 5221829 Fax.: (private) +4940 5228536 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message