From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [PATCH] hush: Fix assignments being misinterpreted as commands
Date: Tue, 2 Mar 2021 08:34:35 -0500 [thread overview]
Message-ID: <20210302133435.GZ10169@bill-the-cat> (raw)
In-Reply-To: <ffdb553d-6dc2-b697-ac93-731310251435@gmail.com>
On Tue, Mar 02, 2021 at 08:24:20AM -0500, Sean Anderson wrote:
> On 3/2/21 8:20 AM, Tom Rini wrote:
> > 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@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?
> >
>
> I'm not changing hush to work like the first two examples, it's already like this.
>
> Only the third example is affected by this patch.
OK. But to what end? Historically we have a buggy but mostly
compatible "hush" that acts like "sh" does. A more flexible shell could
solve a lot of different use cases including making boot scripts that
people end up writing being clearer and easier to write/debug/maintain.
What I worry about here is making our shell not act like any regular
shell people use.
--
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/277c60b0/attachment.sig>
next prev parent reply other threads:[~2021-03-02 13:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-28 21:29 [PATCH] hush: Fix assignments being misinterpreted as commands Sean Anderson
2021-02-28 23:40 ` Heinrich Schuchardt
2021-02-28 23:51 ` Sean Anderson
2021-03-01 14:17 ` Tom Rini
2021-03-01 18:26 ` Heinrich Schuchardt
2021-03-01 23:07 ` Sean Anderson
2021-03-02 13:20 ` Tom Rini
2021-03-02 13:24 ` Sean Anderson
2021-03-02 13:34 ` Tom Rini [this message]
2021-03-02 23:09 ` Sean Anderson
2021-03-03 9:45 ` Wolfgang Denk
2021-03-01 18:43 ` Tom Rini
2021-04-13 14:27 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210302133435.GZ10169@bill-the-cat \
--to=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).