Re: using trap function to cleanup and exit?

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Fri, 2022-04-15 at 15:27 -0700, Bart Schaefer wrote:
> On Thu, Apr 14, 2022 at 2:29 PM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> > 
> > [I] would prefer that another set of eyes review
> > the code change.
> 
> To expand on that a bit ...
> 
> The BIN_RETURN branch checks that (trap_return == -2) which is a
> pretty specific number.  exec.c says:
>  * This is only active if we are inside a trap, else its value
>  * is irrelevant.  It is initialised to -1 for a function trap and
>  * -2 for a non-function trap and if negative is decremented as
>  * we go deeper into functions and incremented as we come back up.
>  * The value is used to decide if an explicit "return" should cause
>  * a return from the caller of the trap; it does this by setting
>  * trap_return to a status (i.e. a non-negative value).
> 
> My interpretation is that, since we are in an explicit "exit" rather
> than an explicit "return", we don't really care how trap_return is
> set; we're going to force the caller to return, period.

I just got back and looked, and it's hard to see how this could make
anything worse.

> However, the test for whether to increment trap_return again on the
> way out is dependent on trap_state == TRAP_STATE_PRIMED, so if we
> change trap_state we're never going to increment (or decrement,
> actually) trap_return again.  (It may, however, get restored from a
> stacked value.)  I can imagine there being some side-effect of this
> from using a mix of "return" and "exit" from traps or from functions
> that might be called by traps, but have no good idea how to exercise
> such a code path.

No, indeed; as long as we accumulate all the tests we can think of for
any complicated cases of combined exit and return I think we're doing as
well as we can.

The only note of caution I can think of is to make sure we're testing
enough of the normal cases as well as the funnies.  As long as that
looks reasonable we can just suck it and see.

pws