This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc] [0/7] infrun cleanup
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: "Ulrich Weigand" <uweigand at de dot ibm dot com>, drow at false dot org
- Date: Sun, 7 Dec 2008 19:15:46 +0000
- Subject: Re: [rfc] [0/7] infrun cleanup
- References: <200812071819.mB7IJnUh004717@d12av02.megacenter.de.ibm.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