hush: do fewer strdups in % and hash expansions

function                                             old     new   delta
builtin_umask                                        133     132      -1
expand_one_var                                      1552    1543      -9

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>

diff --git a/shell/hush.c b/shell/hush.c
index 920d9638c78c711c999ff2e086153631603742fa..3faf2b326c2f9932250b8ba03a95caa0c9687996 100644 (file)
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -3800,8 +3800,8 @@ static int encode_string(o_string *as_string,
                        ch = i_getch(input); /* eat next */
                        if (ch == '\n')
                                goto again; /* skip \<newline> */
-               } /* else: ch remains == '\\', and we double it */
-               o_addqchr(dest, ch); /* \c if c is s glob char, else just c */
+               } /* else: ch remains == '\\', and we double it below: */
+               o_addqchr(dest, ch); /* \c if c is a glob char, else just c */
                nommu_addchr(as_string, ch);
                goto again;
        }
@@ -4618,26 +4618,29 @@ static NOINLINE const char *expand_one_var(char **to_be_freed_pp, char *arg, cha
                         * Then var's value is matched to it and matching part removed.
                         */
                        if (val && val[0]) {
+                               char *t;
                                char *exp_exp_word;
                                char *loc;
                                unsigned scan_flags = pick_scan(exp_op, *exp_word);
                                if (exp_op == *exp_word)        /* ## or %% */
                                        exp_word++;
-//TODO: avoid xstrdup unless needed
-// (see HACK ALERT below for an example)
-                               val = to_be_freed = xstrdup(val);
                                exp_exp_word = encode_then_expand_string(exp_word, /*process_bkslash:*/ 1, /*unbackslash:*/ 1);
                                if (exp_exp_word)
                                        exp_word = exp_exp_word;
-                               loc = scan_and_match(to_be_freed, exp_word, scan_flags);
+                               /* HACK ALERT. We depend here on the fact that
+                                * G.global_argv and results of utoa and get_local_var_value
+                                * are actually in writable memory:
+                                * scan_and_match momentarily stores NULs there. */
+                               t = (char*)val;
+                               loc = scan_and_match(t, exp_word, scan_flags);
                                //bb_error_msg("op:%c str:'%s' pat:'%s' res:'%s'",
-                               //              exp_op, to_be_freed, exp_word, loc);
+                               //              exp_op, t, exp_word, loc);
                                free(exp_exp_word);
                                if (loc) { /* match was found */
                                        if (scan_flags & SCAN_MATCH_LEFT_HALF) /* #[#] */
-                                               val = loc;
+                                               val = loc; /* take right part */
                                        else /* %[%] */
-                                               *loc = '\0';
+                                               val = to_be_freed = xstrndup(val, loc - val); /* left */
                                }
                        }
                }
@@ -4646,13 +4649,13 @@ static NOINLINE const char *expand_one_var(char **to_be_freed_pp, char *arg, cha
                        /* It's ${var/[/]pattern[/repl]} thing.
                         * Note that in encoded form it has TWO parts:
                         * var/pattern<SPECIAL_VAR_SYMBOL>repl<SPECIAL_VAR_SYMBOL>
+                        * and if // is used, it is encoded as \:
+                        * var\pattern<SPECIAL_VAR_SYMBOL>repl<SPECIAL_VAR_SYMBOL>
                         */
                        /* Empty variable always gives nothing: */
                        // "v=''; echo ${v/*/w}" prints "", not "w"
                        if (val && val[0]) {
-                               /* It's ${var/[/]pattern[/repl]} thing */
-                               /*
-                                * Pattern is taken literally, while
+                               /* pattern uses non-standard expansion.
                                 * repl should be unbackslashed and globbed
                                 * by the usual expansion rules:
                                 * >az; >bz;
@@ -7339,7 +7342,7 @@ int hush_main(int argc, char **argv)
        /* Deal with HUSH_VERSION */
        G.shell_ver.flg_export = 1;
        G.shell_ver.flg_read_only = 1;
-       /* Code which handles ${var/P/R} needs writable values for all variables,
+       /* Code which handles ${var<op>...} needs writable values for all variables,
         * therefore we xstrdup: */
        G.shell_ver.varstr = xstrdup(hush_version_str),
        G.top_var = &G.shell_ver;

diff --git a/shell/hush_test/hush-vars/var_bash6.right b/shell/hush_test/hush-vars/var_bash6.right
new file mode 100644 (file)
index 0000000..63fc23d
--- /dev/null
+++ b/shell/hush_test/hush-vars/var_bash6.right
@@ -0,0 +1,5 @@
+Expected Actual
+a*z    : a*z
+\z     : \z
+a1z a2z: a1z a2z
+z      : z

diff --git a/shell/hush_test/hush-vars/var_bash6.tests b/shell/hush_test/hush-vars/var_bash6.tests
new file mode 100755 (executable)
index 0000000..cf2e4f0
--- /dev/null
+++ b/shell/hush_test/hush-vars/var_bash6.tests
@@ -0,0 +1,9 @@
+# This testcase checks globbing correctness in ${v/a/b}
+
+>a1z; >a2z;
+          echo 'Expected' 'Actual'
+v='a bz'; echo 'a*z    :' "${v/a*z/a*z}"
+v='a bz'; echo '\z     :' "${v/a*z/\z}"
+v='a bz'; echo 'a1z a2z:' ${v/a*z/a*z}
+v='a bz'; echo 'z      :' ${v/a*z/\z}
+rm a1z a2z