Re: [PATCH v3 06/17] Use keep_going in proceed and start_step_over too

Pedro Alves
2015-04-22T22:22:35Z

On 04/22/2015 06:08 AM, Doug Evans wrote:
> Pedro Alves writes:
> > The main motivation of this patch is sharing more code between the
> > proceed (starting the inferior for the first time) and keep_going
> > (restarting the inferior after handling an event) paths and using the
> > step_over_chain queue now embedded in the thread_info object for
> > pending in-line step-overs too (instead of just for displaced
> > stepping).
>
> Hi.
> A couple of nits inline.
> grep for ====
>

Thanks.

> > + /* keep_going_pass skips the step-over if the breakpoint is no
> > + longer inserted. In all-stop, we want to keep looking for a
>
> ====
> missing word? "... keep looking for a thread" ?

Indeed.

> > - if (!tp->control.in_infcall)
> > - set_running (user_visible_resume_ptid (user_step), 1);
> > + if (debug_infrun)
> > + fprintf_unfiltered (gdb_stdlog,
> > + "Got placed in displaced stepping queue\n");
>
> ====
> Nit: Suggest rewording "displaced stepping queue".
> IIUC there is no displaced-stepping specific queue anymore.

Fixed:

+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "Got placed in step-over queue\n");

> > -/* Called when we should continue running the inferior, because the
> > - current event doesn't cause a user visible stop. This does the
> > - resuming part; waiting for the next event is done elsewhere. */
> > +/* Like keep_going, but passes the signal to the inferior, even if the
> > + signal is set to nopass. */
> >
> > static void
> > -keep_going (struct execution_control_state *ecs)
> > +keep_going_pass (struct execution_control_state *ecs)
>
> ====
> For whatever weird reasons, "keep_going_pass" doesn't read very well to me.
> "pass" as in "pass/fail"?
> "pass" as in one of several passes, and this is the "keep going" one?
>
> Can you rename this to keep_going_pass_signal?

No problem. Done that.

Thanks,
Pedro Alves