[PATCH] shell script error management in ash (set of 6 patches)

[Roberto A. Foglietta]

Il giorno mar 24 ago 2021 alle ore 23:42 Harald van Dijk <harald at gigawatt.nl>
ha scritto:

> On 24/08/2021 11:10, Roberto A. Foglietta wrote:
> > Thanks again for your insightfulness.
> >
> > I reverted back to the previous situation when I was resetting the
> > recursive flag when the evaltree returns:
> >
> >   - if evaltree returns: no problem
> >   - before an not-exit not-returning exception is raised, it sets an
> > handler that makes it returning
> >
> >   Moreover, I used static variables to save the previous state in such a
> > way I am able to set back again in case my handler takes off.
> >   I have updated the testsuite and moved the test9.sh in testD.sh plus
> > testD.sh makes the check of the last case you arise to my attention.
> >
> >   The full patch is here:
> >
> > https://github.com/robang74/tinycore-editor/tree/main/busybox/patches
> > <https://github.com/robang74/tinycore-editor/tree/main/busybox/patches>
> >   busybox-1.33.1-error-management-extension-for-ash-v017.patch
>
> I had trouble applying the patch you attached, so I reset everything and
> restarted from that full patch. I'm getting:
>
>    shell/ash.c: In function ‘evalfun’:
>    shell/ash.c:9910:16: warning: variable ‘evaldone’ set but not used
> [-Wunused-but-set-variable]
>     9910 |         int e, evaldone = 0;
>          |                ^~~~~~~~
>
> That looks like a correct warning, evaldone is never read, only assigned
> to.
>

Debug code that remains in the patch, removed.


>
> As for the handling of errors, you are now catching EXERROR and not
> propagating the error. That does not look right.


It is necessary otherwise testD.sh fails


> However, when trying to
> create a testcase for that, I stumbled upon a more fundamental problem:
>
>    f() {
>      trap "echo ERR" ERR
>      false
>    }
>    f
>
> This is supposed to print ERR. You've implemented set +E (the default
> mode) as forcibly disabling ERR handlers inside functions, but that's
> not what it's supposed to do, nor what it's documented as doing in bash.
> What it's supposed to do is only to prevent outer ERR handlers from
> being inherited, it's not supposed to prevent ERR handlers defined
> within the function from taking effect.
>

Thanks, you are right. I reworked the set -E again. In attachment the patch
and the new test suite.

Please check obash.txt versus bbash.txt in the testsuite because bash
behaves slightly differently. It seems to me that sometimes bash is
incoherent and patched ash behaves in an acceptable way but I want a second
opinion on this.

 The full patch is here:

 https://github.com/robang74/tinycore-editor/tree/main/busybox/patches
 busybox-1.33.1-error-management-extension-for-ash-v018.patch

 Please note that the full patch is a collection of patches, actually.
 I am going to change it when we reach an "OK GO" state.

 Best regards,
-- 
Roberto A. Foglietta
+39.349.33.30.697
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20210825/3631421c/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox-1.33.1-set-E-second-rework-and-minors.patch
Type: application/octet-stream
Size: 1932 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20210825/3631421c/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testsuite.tgz
Type: application/x-compressed
Size: 3746 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20210825/3631421c/attachment-0001.bin>