Don't exit with zero in the trap handler

When configure runs "exit 1", the trap handler is run to cleanup any
files created by configure, but this trap handler itself calls "exit"
with no argument (which means zero exit code):
[...]
+ echo Error: zlib check failed
Error: zlib check failed
+ echo Make sure to have the zlib libs and headers installed.
Make sure to have the zlib libs and headers installed.
+ echo

+ exit 1
+ rm -f /tmp/qemu-conf--2779-.c /tmp/qemu-conf--2779-.o
/tmp/qemu-conf--2779-.exe
+ exit

To fix this, remove the call to exit from the trap handler, leaving it
to the shell shell to exitafter the trap handler is run (honoring any
previously provided exit code).
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Loïc Minier <loic.minier@linaro.org> writes:

> When configure runs "exit 1", the trap handler is run to cleanup any
> files created by configure, but this trap handler itself calls "exit"
> with no argument (which means zero exit code):
> [...]
> + echo Error: zlib check failed
> Error: zlib check failed
> + echo Make sure to have the zlib libs and headers installed.
> Make sure to have the zlib libs and headers installed.
> + echo
>
> + exit 1
> + rm -f /tmp/qemu-conf--2779-.c /tmp/qemu-conf--2779-.o
> /tmp/qemu-conf--2779-.exe
> + exit
>
> To fix this, remove the call to exit from the trap handler, leaving it
> to the shell shell to exitafter the trap handler is run (honoring any
> previously provided exit code).

This suggests the old code screws up the exit code.  It doesn't for me.
Unless it does at least on some platforms, it's a cleanup, not a fix,
and the commit message should reflect that.

On Sat, Sep 25, 2010, Markus Armbruster wrote:
> This suggests the old code screws up the exit code.  It doesn't for me.
> Unless it does at least on some platforms, it's a cleanup, not a fix,
> and the commit message should reflect that.

 It does screw up the exit code for me; it seems it's because dash is
 used as /bin/sh.  If I call this shell snippet:
    trap "echo trap; exit" 0 1 2 3 9 11 13 15
    exit 2
 with dash, e.g. "dash foo.sh; echo $?", I get 0, and with bash I get 2.

 I'm not sure what POSIX says, but given that calling exit in a trap
 handler isn't needed here, I recommend including this as a bug fix.

On Sat, Sep 25, 2010 at 8:31 AM, Loïc Minier <lool@dooz.org> wrote:
> On Sat, Sep 25, 2010, Markus Armbruster wrote:
>> This suggests the old code screws up the exit code.  It doesn't for me.
>> Unless it does at least on some platforms, it's a cleanup, not a fix,
>> and the commit message should reflect that.
>
>  It does screw up the exit code for me; it seems it's because dash is
>  used as /bin/sh.  If I call this shell snippet:
>    trap "echo trap; exit" 0 1 2 3 9 11 13 15
>    exit 2
>  with dash, e.g. "dash foo.sh; echo $?", I get 0, and with bash I get 2.
>
>  I'm not sure what POSIX says, but given that calling exit in a trap
>  handler isn't needed here, I recommend including this as a bug fix.

http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_21

"When exit is executed in a trap action, the last command is
considered to be the command that executed immediately preceding the
trap action."

It looks like dash and ksh are not compliant and use the return value
of echo or rm inside trap handler:
dash -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
4
ksh -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
4
bash -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
3

On Sat, Sep 25, 2010, Blue Swirl wrote:
> It looks like dash and ksh are not compliant and use the return value
> of echo or rm inside trap handler:
> dash -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
> 4

 I've filed https://bugs.launchpad.net/dash/+bug/647450 to track this
 and forwarded the bug to dash@vger.kernel.org.

> ksh -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
> 4

 On my system, ksh is provided by zsh and zsh gets this right.


 In the mean time, could you please pull the patch in QEMU?  Without
 "exit" in the trap handler, the correct exit code will be returned.

    Thanks,

Loïc Minier <lool@dooz.org> writes:

> On Sat, Sep 25, 2010, Blue Swirl wrote:
>> It looks like dash and ksh are not compliant and use the return value
>> of echo or rm inside trap handler:
>> dash -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
>> 4
>
>  I've filed https://bugs.launchpad.net/dash/+bug/647450 to track this
>  and forwarded the bug to dash@vger.kernel.org.
>
>> ksh -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
>> 4
>
>  On my system, ksh is provided by zsh and zsh gets this right.

So it's a cleanup that also works around a bug in dash.

>  In the mean time, could you please pull the patch in QEMU?  Without
>  "exit" in the trap handler, the correct exit code will be returned.

Would you mind resending with a more suitable commit message?

On Sat, Sep 25, 2010 at 12:01 PM, Loïc Minier <lool@dooz.org> wrote:
> On Sat, Sep 25, 2010, Blue Swirl wrote:
>> It looks like dash and ksh are not compliant and use the return value
>> of echo or rm inside trap handler:
>> dash -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
>> 4
>
>  I've filed https://bugs.launchpad.net/dash/+bug/647450 to track this
>  and forwarded the bug to dash@vger.kernel.org.
>
>> ksh -c 'trap "sh -c \"exit 4\"; exit" 0 1 2 3 9 11 13 15;exit 3'; echo $?
>> 4
>
>  On my system, ksh is provided by zsh and zsh gets this right.

On OpenBSD, ksh is pdksh and it fails like dash.

>  In the mean time, could you please pull the patch in QEMU?  Without
>  "exit" in the trap handler, the correct exit code will be returned.

I tried also saving the return value, but it does not work:
dash -c 'trap "ret=$?;echo ret: $ret;sh -c \"exit 4\";exit $ret" 0 1 2
3 9 11 13 15;sh -c "exit 5"; echo exit now $?;exit'; echo $?
exit now 5
ret:
4
ksh -c 'trap "ret=$?;echo ret: $ret;sh -c \"exit 4\";exit $ret" 0 1 2
3 9 11 13 15;sh -c "exit 5"; echo exit now $?;exit'; echo $?
exit now 5
ret:
4
bash -c 'trap "ret=$?;echo ret: $ret;sh -c \"exit 4\";exit $ret" 0 1 2
3 9 11 13 15;sh -c "exit 5"; echo exit now $?;exit'; echo $?
exit now 5
ret:
0

Perhaps a short comment should be added to warn against adding exit.