Re: eval: Do not cache value of eflag in evaltree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2021-05-17 at 15:19 +0800, Herbert Xu wrote:
> Patrick Brünn <P.Bruenn@xxxxxxxxxxxx> wrote:
> >
> > Since we are migrating to Debian bullseye, we discovered a new behavior
> > with our scripts, which look like this:
> > > #!/bin/sh
> > > cleanup() {
> > >        set +e^M
> > >        rmdir ""
> > > }
> > > set -eu
> > > trap 'cleanup' EXIT INT TERM
> > > echo 'Hello world!'
> >
> > With old dash v0.5.10.2 this script would return 0 as we expected it.
> > But since commit 62cf6955f8abe875752d7163f6f3adbc7e49ebae it returns
> > the last exit code of our cleanup function.
> > Reverting that commit gives a merge conflict, but it seems to fix _our_
> > problem. As that topic appears too complex to us I want to ask the
> > experts here:
> >
> > Is this change in behavior intended, by dash?
> >
> > Our workaround at the moment would be:
> > > trap 'cleanup || true' EXIT INT TERM
>
> Thanks for the report.  This is actually a fairly old bug with
> set -e that's just been exposed by the exit status change.  What's
> really happening is that cleanup itself is triggering a set -e
> exit incorrectly because evaltree cached the value of eflag prior
> to the function call.
>
> This patch should fix the problem.
>
> Reported-by: Patrick Brünn <P.Bruenn@xxxxxxxxxxxx>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
>
> diff --git a/src/eval.c b/src/eval.c
> index 9476fbb..3337f71 100644
> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -252,18 +252,10 @@ evaltree(union node *n, int flags)
>                         popredir(0);
>                 goto setstatus;
>         case NCMD:
> -#ifdef notyet
> -               if (eflag && !(flags & EV_TESTED))
> -                       checkexit = ~0;
> -               status = evalcommand(n, flags, (struct backcmd *)NULL);
> -               goto setstatus;
> -#else
>                 evalfn = evalcommand;
>  checkexit:
> -               if (eflag && !(flags & EV_TESTED))
> -                       checkexit = ~0;
> +               checkexit = ~flags & EV_TESTED;
>                 goto calleval;
> -#endif
>         case NFOR:
>                 evalfn = evalfor;
>                 goto calleval;
> @@ -323,7 +315,7 @@ setstatus:
>  out:
>         dotrap();
>
> -       if (checkexit & status)
> +       if (eflag && checkexit && status)
>                 goto exexit;
>
>         if (flags & EV_EXIT) {
> --
Working for us, thanks!
Tested-by: Patrick Brünn <P.Bruenn@xxxxxxxxxxxx>
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075





[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux