From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Mon, 1 Mar 2021 18:07:36 -0500 Subject: [PATCH] hush: Fix assignments being misinterpreted as commands In-Reply-To: References: <20210228212951.1175231-1-seanga2@gmail.com> <914D3972-B8A7-45A8-AC3E-95519C56EEB7@gmx.de> <20210301141705.GZ10169@bill-the-cat> Message-ID: <4e476358-e511-90de-fd47-ad24ddeecd2e@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 : >>>>> 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' 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. --Sean