From: Sean Anderson <seanga2@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH] hush: Fix assignments being misinterpreted as commands
Date: Tue, 2 Mar 2021 08:24:20 -0500 [thread overview]
Message-ID: <ffdb553d-6dc2-b697-ac93-731310251435@gmail.com> (raw)
In-Reply-To: <20210302132039.GT10169@bill-the-cat>
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.
--Sean
next prev parent reply other threads:[~2021-03-02 13:24 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 [this message]
2021-03-02 13:34 ` Tom Rini
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=ffdb553d-6dc2-b697-ac93-731310251435@gmail.com \
--to=seanga2@gmail.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).