[PATCH] hush: Fix assignments being misinterpreted as commands

[Tom Rini]

On Mon, Mar 01, 2021 at 06:07:36PM -0500, Sean Anderson wrote:
> On 3/1/21 1:26 PM, Heinrich Schuchardt wrote:
> > On 3/1/21 3:17 PM, Tom Rini wrote:
> > > On Sun, Feb 28, 2021 at 06:51:53PM -0500, Sean Anderson wrote:
> > > > On 2/28/21 6:40 PM, Heinrich Schuchardt wrote:
> > > > > Am 28. Februar 2021 22:29:51 MEZ schrieb Sean Anderson <seanga2 at gmail.com>:
> > > > > > If there were no variable substitutions in a command, then initial
> > > > > > assignments would be misinterpreted as commands, instead of being
> > > > > > skipped
> > > > > > over. This is demonstrated by the following example:
> > > > > > 
> > > > > >     => foo=bar echo baz
> > > > > 
> > > > > The commit message does not explain why this patch is needed.
> > > > 
> > > > This is a bug I noticed while writing some tests of hush.
> > > > 
> > > > > What shall be the value off foo after this line?
> > > > 
> > > > It should be bar. This is an existing difference when compared with
> > > > bash. For example, without this patch, we have
> > > > 
> > > >     => foo=bar echo $foo
> > > >     bar
> > > >     => echo $foo
> > > >     bar
> > 
> > This seems really awkward. In bash I get:
> > 
> > $ foo=bar ./test.sh
> > bar
> > $ echo $foo
> > 
> > $
> > 
> > Where test.sh
> > 
> > #!/bin/sh
> > echo $foo
> > 
> > I did not expect an assignment made before a command to stick.
> 
> Yeah, this is because hush does not have the concept of per-command
> assignments (scope). So everything happens in the global scope.
> 
> > > > 
> > > > > 
> > > > > What will be the output of
> > > > > 
> > > > > foo=bar echo ${foo}
> > > > > 
> > > > > with and without your patch?
> > > > 
> > > > It is the same.
> > 
> > Please, provide an example where the patch makes a difference.
> > 
> > Best regards
> > 
> > Heinrich
> > 
> > > 
> > > bash works as you describe.  dash and busybox-sh both function like
> > > this:
> > > $ foo=bar echo $foo
> > > 
> > > $ echo $foo
> > > 
> > > $
> > > 
> > > That we error out entirely is different from everyone.  Is that a good
> > > thing?  Maybe.  I know I've caught myself making thinkos due to that
> > > logic.  It does also violate the principal of least surprise, that we
> > > don't act like anything else.  But I would suggest the behavior of
> > > busybox-sh (what we forked long long ago) is what we should model here
> > > rather than be more bash-like.  I'm not all that firm on this opinion
> > > frankly, especially given the one-line nature of the change to bring us
> > > that behavior and I assume dash/busybox are acting like pure sh would in
> > > this case, which we aren't anyhow.
> 
> Ok, I'd like to clear things up. Here is the current behavior of U-Boot:
> 
> 
> 	=> foo=bar echo $foo
> 	bar
> 	=> echo $foo
> 	bar
> 	=> baz=bar echo qux
> 	Unknown command 'baz=bar' - try 'help'

I see this as well (which isn't what I said in another part of this
thread, so I re-checked just now).

> with this patch, this changes to
> 
> 	=> foo=bar echo $foo
> 	bar
> 	=> echo $foo
> 	bar
> 	=> baz=bar echo qux
> 	qux
> 
> This patch *only* affects cases where there is an assignment at the
> beginning of the line, but there is *no* variable reference in the
> command. I know this is an edge case, but the current logic is clearly
> wrong here.

OK.  But I don't see that behavior in bash 4.4.20 (Ubuntu 18.04).  Where
does one get a shell that works like you're changing our hush to?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210302/c9fc4641/attachment.sig>