Re: [rfc] [0/7] infrun cleanup

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 07 December 2008 18:19:49, Ulrich Weigand wrote:

> In the context of some further cleanup and splitting handle_inferior_event
> into multiple more independent parts, I had been wondering whether it
> might be a good idea to use a enum (like enum inferior_stop_reason) 
> instead of the boolean: handle_inferior_event (and its hypothetical
> subroutines) would return enum values to indicate *why* the inferior
> stopped, including a new STILL_RUNNING value to indicate that it
> in fact hasn't yet stopped.
> 
> In this set-up you'd have statements like
> 
>    return STILL_RUNNING;

'return WAIT_SOME_MORE;' would be historically more appropriate.  :-)

> 
> or
> 
>    return END_STEPPING_RANGE;
> 
> or (another potential new value)
> 
>    return HIT_BREAKPOINT;
> 
> within handle_inferior_event; and its caller would be rewritten like
> 
>   do
>     {
>       ptid = target_wait (...);
>       stop_reason = handle_inferior_event (ptid, ...);
>     }
>   while (stop_reason == STILL_RUNNING);
> 
> It might be feasible to use the stop_reason in the future to merge
> some of the print_stop_reason stuff into normal_stop and reduce the
> amount of duplicate checks; or even to replace some of the "global"
> output variables like stopped_by_random_signal or tp->stop_step.

Sounds good to me.


> I thought I had updated that one?

Oh, you did.  I missed it, sorry.

> 
> >   /* Refresh prev_pc value just prior to resuming.  This used to be
> >      done in stop_stepping, however, setting prev_pc there did not handle
> >      scenarios such as inferior function calls or returning from
> >      a function via the return command.  In those cases, the prev_pc
> >      ...
> 
> I left that because it specifically refers to how things were done
> in the past, when we still had that function ...  Maybe it could be
> marked as "the former stop_stepping" or so.

Well, that whole comment becomes mostly useless after these cleanups.
It refers to stop_stepping, ecs, init_execution_control_state, all things
that are gone, which seems to leave more confusion than clarify things.
I wonder if we should just remove most of it, and just comment why we
set prev_pc there...

-- 
Pedro Alves