[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] ash: avoid leaking memory associated with redirections. Closes 7748
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2019-02-25 17:58:57
Message-ID: CAK1hOcP69zrJms-n4EmNaaQQOAq2Od9eF2X5uGO1tvcZ-=fqGQ () mail ! gmail ! com
[Download RAW message or body]

Applied, thanks!

On Mon, Feb 25, 2019 at 9:29 AM Ron Yorston <rmy@pobox.com> wrote:
>
> The following constructs result in ever-increasing memory usage:
>
>    while true; do { true; } </dev/null; done
>    while true; do ( true; ) </dev/null; done
>
> For comparison, bash displays static memory usage in both cases.
>
> This has been fixed in dash by commit 2bc6caa.  The maintainer
> writes:
>
>    I have simplified evaltree so that it simply sets the stack mark
>    unconditionally.  This allows us to remove the stack marks in the
>    functions called by evaltree.
>
> Closes BusyBox bug 7748.
>
> function                                             old     new   delta
> evaltree                                             606     632     +26
> evalcommand                                         1724    1696     -28
> evalcase                                             382     351     -31
> evalfor                                              230     196     -34
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 1/3 up/down: 26/-93)            Total: -67 bytes
>
> Signed-off-by: Ron Yorston <rmy@pobox.com>
> ---
>  shell/ash.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index a284b084d..a4394d756 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -9034,8 +9034,11 @@ evaltree(union node *n, int flags)
>  {
>         int checkexit = 0;
>         int (*evalfn)(union node *, int);
> +       struct stackmark smark;
>         int status = 0;
>
> +       setstackmark(&smark);
> +
>         if (n == NULL) {
>                 TRACE(("evaltree(NULL) called\n"));
>                 goto out;
> @@ -9149,6 +9152,7 @@ evaltree(union node *n, int flags)
>         if (flags & EV_EXIT)
>                 raise_exception(EXEXIT);
>
> +       popstackmark(&smark);
>         TRACE(("leaving evaltree (no interrupts)\n"));
>         return exitstatus;
>  }
> @@ -9209,14 +9213,12 @@ evalfor(union node *n, int flags)
>         struct arglist arglist;
>         union node *argp;
>         struct strlist *sp;
> -       struct stackmark smark;
>         int status = 0;
>
>         errlinno = lineno = n->ncase.linno;
>         if (funcline)
>                 lineno -= funcline - 1;
>
> -       setstackmark(&smark);
>         arglist.list = NULL;
>         arglist.lastp = &arglist.list;
>         for (argp = n->nfor.args; argp; argp = argp->narg.next) {
> @@ -9233,7 +9235,6 @@ evalfor(union node *n, int flags)
>                         break;
>         }
>         loopnest--;
> -       popstackmark(&smark);
>
>         return status;
>  }
> @@ -9244,14 +9245,12 @@ evalcase(union node *n, int flags)
>         union node *cp;
>         union node *patp;
>         struct arglist arglist;
> -       struct stackmark smark;
>         int status = 0;
>
>         errlinno = lineno = n->ncase.linno;
>         if (funcline)
>                 lineno -= funcline - 1;
>
> -       setstackmark(&smark);
>         arglist.list = NULL;
>         arglist.lastp = &arglist.list;
>         expandarg(n->ncase.expr, &arglist, EXP_TILDE);
> @@ -9270,8 +9269,6 @@ evalcase(union node *n, int flags)
>                 }
>         }
>   out:
> -       popstackmark(&smark);
> -
>         return status;
>  }
>
> @@ -9970,7 +9967,6 @@ evalcommand(union node *cmd, int flags)
>         struct localvar_list *localvar_stop;
>         struct parsefile *file_stop;
>         struct redirtab *redir_stop;
> -       struct stackmark smark;
>         union node *argp;
>         struct arglist arglist;
>         struct arglist varlist;
> @@ -9992,7 +9988,6 @@ evalcommand(union node *cmd, int flags)
>
>         /* First expand the arguments. */
>         TRACE(("evalcommand(0x%lx, %d) called\n", (long)cmd, flags));
> -       setstackmark(&smark);
>         localvar_stop = pushlocalvars();
>         file_stop = g_parsefile;
>         back_exitstatus = 0;
> @@ -10275,7 +10270,6 @@ evalcommand(union node *cmd, int flags)
>                  */
>                 setvar0("_", lastarg);
>         }
> -       popstackmark(&smark);
>
>         return status;
>  }
> --
> 2.20.1
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]