Date: Thu, 20 Apr 2000 12:19:43 +0200 From: Martin Cracauer <cracauer@cons.org> To: Anatoly Vorobey <mellon@pobox.com> 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> In-Reply-To: <200004200630.XAA07459@freefall.freebsd.org>; from mellon@pobox.com on Wed, Apr 19, 2000 at 11:30:02PM -0700 References: <200004200630.XAA07459@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <mellon@pobox.com>
> 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 <cracauer@cons.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000420121942.A14798>