public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V2] hush: fix some quoted variable expansion issues
Date: Sat, 01 Mar 2014 21:26:26 -0700	[thread overview]
Message-ID: <5312B2F2.1070000@wwwdotorg.org> (raw)
In-Reply-To: <CAPnjgZ0rBFV=CbOJ1G8WgUi1fH2=apKA1Q3i8Pd5_Z_ag_2s0Q@mail.gmail.com>

On 03/01/2014 05:10 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On 27 February 2014 22:00, Stephen Warren <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     The following shell command fails:
> 
>     if test -z "$x"; then echo "zero"; else echo "non-zero"; fi
> 
>     (assuming $x does not exist, it prints "non-zero" rather than "zero").
> 
>     ... since "$x" expands to nothing, and the argument is completely
>     dropped, causing too few to be passed to -z, causing cmd_test() to
>     error out early.
> 
>     This is because when variable expansions are processed by make_string(),
>     the expanded results are concatenated back into a new string. However,
>     no quoting is applied when doing so, so any empty variables simply don't
>     generate any parameter when the combined string is parsed again.
> 
>     Fix this by explicitly replacing quoting any argument that was
>     originally
>     quoted when re-generating a string from the already-parsed argument
>     list.
> 
>     This also fixes loss of whitespace in commands such as:
> 
>     setenv space " "
>     setenv var " 1${space}${space} 2 "
>     echo ">>${var}<<"
> 
> 
> Is there an upstream still for hush, or are we so far away that it
> doesn't matter? If there is, was this bug fixed there?

Well, the comments at the head of the file say it came from Busybox, but
it's obviously diverged massively since it was imported:

$ wc -l busybox/shell/hush.c u-boot/common/hush.c
9156 busybox/shell/hush.c
3682 u-boot/common/hush.c
$ diff -u busybox/shell/hush.c u-boot/common/hush.c|wc -l
12264

Also, the function this patch touches doesn't seem to exist under that
name any more. From a quick look at the source, I couldn't tell what the
equivalent is.

Perhaps replaying patches to that file in Busybox since the fork might
be more useful.

A quick Google search for "hush" or "hush shell" doesn't seem to yield
any other alternative upstreams.

>     diff --git a/common/hush.c b/common/hush.c
>     index 3f3a79c..66ece41 100644
>     --- a/common/hush.c
>     +++ b/common/hush.c
>     @@ -221,6 +221,7 @@ struct child_prog {
>             pid_t pid;                                      /* 0 if
>     exited */
>      #endif
>             char **argv;                            /* program name and
>     arguments */
>     +       int *argv_nonnull;
> 
> 
> This could do with a comment.

I did wonder if I should name this "quoted". I'd originally shied away
from this to prevent changing the terminology, but since hush.c has
diverged so much, perhaps that wouldn't be an issue...

  parent reply	other threads:[~2014-03-02  4:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28  5:00 [U-Boot] [PATCH V2] hush: fix some quoted variable expansion issues Stephen Warren
2014-03-02  0:10 ` Simon Glass
2014-03-02  0:16   ` Simon Glass
2014-03-02  4:26   ` Stephen Warren [this message]
2014-03-03 14:28     ` Tom Rini
2014-03-02  5:13   ` Stephen Warren

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=5312B2F2.1070000@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --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