[PATCH] ash.c: speedup ${s:} substring (no quotes)

[Denys Vlasenko]

On Wed, Jul 28, 2021 at 10:40 AM Alin Mr <almr.oss at outlook.com> wrote:
> This trivial patch makes ${s:...} at least as fast as ${s#??..} in
> simple tests. It's probably faster for longer substrings, but then one
> wouldn't use ${s#"1024???s"} anyway -- one would switch away from sh.
> memmove could possibly be replaced by memcpy.
>
> I haven't found much on testing (or contributing for that matter) on the
> website or in the source. `make test` produces a bunch of failures on
> master anyway, so I'm not sure how to test new code.

cd shell/ash_test

./run-all


> Signed-off-by: Alin Mr <almr.oss at outlook.com>
> ---
>   shell/ash.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index b5947147a..df4d82e39 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -7198,14 +7198,25 @@ subevalvar(char *start, char *str, int strloc,
>                 if ((unsigned)len > (orig_len - pos))
>                         len = orig_len - pos;
>
> -               for (vstr = startp; pos; vstr++, pos--) {
> -                       if (quotes && (unsigned char)*vstr == CTLESC)
> -                               vstr++;
> +               if (! quotes) {
> +                       vstr = startp + pos;
> +                       pos = 0;

"pos = 0" is useless

> +               } else {
> +                       for (vstr = startp; pos; vstr++, pos--) {
> +                               if ((unsigned char)*vstr == CTLESC)
> +                                       vstr++;
> +                       }
>                 }
> -               for (loc = startp; len; len--) {
> -                       if (quotes && (unsigned char)*vstr == CTLESC)
> +               if (! quotes) {

You can also merge if (!quotes) {} blocks - why test the condition twice?

> +                       memmove (loc, vstr, len);

Should be memmove(startp, vstr, len).

> +                       loc = startp + len;
> +                       len = 0;

"len = 0" is useless.

> +               } else {
> +                       for (loc = startp; len; len--) {
> +                               if ((unsigned char)*vstr == CTLESC)
> +                                       *loc++ = *vstr++;
>                                 *loc++ = *vstr++;
> -                       *loc++ = *vstr++;
> +                       }
>                 }
>                 *loc = '\0';
>                 goto out;

Applied in this form:

                if (!quotes) {
                        loc = mempcpy(startp, startp + pos, len);
                } else {
                        for (vstr = startp; pos != 0; pos--) {
                                if ((unsigned char)*vstr == CTLESC)
                                        vstr++;
                                vstr++;
                        }
                        for (loc = startp; len != 0; len--) {
                                if ((unsigned char)*vstr == CTLESC)
                                        *loc++ = *vstr++;
                                *loc++ = *vstr++;
                        }
                }

Thank you.