Re: [U-Boot] [PATCH] hush: when a variable expansion is empty, don't drop the parameter

2014-02-27 Thread Russell King - ARM Linux
On Wed, Feb 26, 2014 at 11:55:06PM -0700, Stephen Warren 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 any empty expansion with an empty pair
 of quotes, so that an argument is still generated.
 
 Reported-by: Russell King li...@arm.linux.org.uk
 Signed-off-by: Stephen Warren swar...@wwwdotorg.org
 ---
 Hmm. I wonder if this causes other problems, such as:
 
 if test -z $x; then echo zero; else echo non-zero; fi
 
 I guess we should only quote the expansion if the input string had quotes
 around the text being expanded, but I'm not sure if we can know that.
 Perhaps more investigation is needed:-( Although... I /guess/ that case
 is rarer, so this patch still improves things?

Standard shell will only drop an empty expansion if it's unquoted, so:

a=1 2 3
b=4
c=

echo $a $b $c

ends up with arg1 = 1, arg2 = 2, arg3 = 3, arg4 = 4 and no arg5

echo $a $b $c

ends up with arg1 = 1 2 3, arg2 = 4, arg3 =  and no arg4

echo $a $b $c

ends up with arg1 = 1 2 3 4  and no arg2

If you'd like to check out this behaviour, this script will help you see
each argument as it was passed by the shell.

#!/bin/sh
n=1
for a in $@; do
  echo arg$n: \$a\
  n=$(($n + 1))
done

Getting this right matters, because it affects stuff like the ability
to make use of test -z / test -n, while preserving the ability to have
unquoted empty variables disappear from argument lists.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] hush: when a variable expansion is empty, don't drop the parameter

2014-02-27 Thread Russell King - ARM Linux
On Thu, Feb 27, 2014 at 10:39:24AM +, Russell King - ARM Linux wrote:
 Standard shell will only drop an empty expansion if it's unquoted, so:
 
   a=1 2 3
   b=4
   c=
 
   echo $a $b $c
 
 ends up with arg1 = 1, arg2 = 2, arg3 = 3, arg4 = 4 and no arg5
 
   echo $a $b $c
 
 ends up with arg1 = 1 2 3, arg2 = 4, arg3 =  and no arg4
 
   echo $a $b $c
 
 ends up with arg1 = 1 2 3 4  and no arg2
 
 If you'd like to check out this behaviour, this script will help you see
 each argument as it was passed by the shell.
 
 #!/bin/sh
 n=1
 for a in $@; do
   echo arg$n: \$a\
   n=$(($n + 1))
 done
 
 Getting this right matters, because it affects stuff like the ability
 to make use of test -z / test -n, while preserving the ability to have
 unquoted empty variables disappear from argument lists.

There's another case where hush goes wrong:

echo Longblah: $foo
echo Blah: $foo

should produce output where the :'s are aligned.  Does in standard shells,
but not in hush.  I suspect this is because the word splitting/expansion
isn't quite correct.  Words inside quotes should not be split even after
expansion - unless it's an expansion of @.  Note that also fixes the
$emptyvariable problem at the root cause of the test -n/-z.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] hush: when a variable expansion is empty, don't drop the parameter

2014-02-26 Thread Stephen Warren
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 any empty expansion with an empty pair
of quotes, so that an argument is still generated.

Reported-by: Russell King li...@arm.linux.org.uk
Signed-off-by: Stephen Warren swar...@wwwdotorg.org
---
Hmm. I wonder if this causes other problems, such as:

if test -z $x; then echo zero; else echo non-zero; fi

I guess we should only quote the expansion if the input string had quotes
around the text being expanded, but I'm not sure if we can know that.
Perhaps more investigation is needed:-( Although... I /guess/ that case
is rarer, so this patch still improves things?
---
 common/hush.c | 8 ++--
 test/command_ut.c | 6 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 3f3a79c..7a67706 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -116,7 +116,7 @@
 #endif
 #endif
 #define SPECIAL_VAR_SYMBOL 03
-#define SUBSTED_VAR_SYMBOL 04
+#define q 04
 #ifndef __U_BOOT__
 #define FLAG_EXIT_FROM_LOOP 1
 #define FLAG_PARSE_SEMICOLON (1  1)  /* symbol ';' is special for 
parser */
@@ -3594,12 +3594,15 @@ static char * make_string(char ** inp)
int len = 2;
char *noeval_str;
int noeval = 0;
+   char *empty = \\;
 
noeval_str = get_local_var(HUSH_NO_EVAL);
if (noeval_str != NULL  *noeval_str != '0'  *noeval_str != '\0')
noeval = 1;
for (n = 0; inp[n]; n++) {
p = insert_var_value_sub(inp[n], noeval);
+   if (!*p)
+   p = empty;
str = xrealloc(str, (len + strlen(p)));
if (n) {
strcat(str,  );
@@ -3608,7 +3611,8 @@ static char * make_string(char ** inp)
}
strcat(str, p);
len = strlen(str) + 3;
-   if (p != inp[n]) free(p);
+   if (p != inp[n]  p != empty)
+   free(p);
}
len = strlen(str);
*(str + len) = '\n';
diff --git a/test/command_ut.c b/test/command_ut.c
index 620a297..6074b0e 100644
--- a/test/command_ut.c
+++ b/test/command_ut.c
@@ -137,6 +137,12 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
HUSH_TEST(or_1_0_inv_inv, ! ! aaa = aaa -o ! ! bbb != bbb, y);
HUSH_TEST(or_1_1_inv_inv, ! ! aaa = aaa -o ! ! bbb = bbb, y);
 
+   run_command(setenv ut_var_nonexistent, 0);
+   run_command(setenv ut_var_exists 1, 0);
+   HUSH_TEST(z_varexp, -z \$ut_var_nonexistent\, y);
+   HUSH_TEST(z_varexp, -z \$ut_var_exists\, n);
+   run_command(setenv ut_var_exists, 0);
+
 #ifdef CONFIG_SANDBOX
/*
 * File existence
-- 
1.8.3.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot