This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 06/17] Use keep_going in proceed and start_step_over too
- From: Pedro Alves <palves at redhat dot com>
- To: Doug Evans <dje at google dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 22 Apr 2015 23:22:29 +0100
- Subject: Re: [PATCH v3 06/17] Use keep_going in proceed and start_step_over too
- Authentication-results: sourceware.org; auth=none
- References: <1429267521-21047-1-git-send-email-palves at redhat dot com> <1429267521-21047-7-git-send-email-palves at redhat dot com> <21815 dot 11476 dot 327167 dot 153546 at ruffy2 dot mtv dot corp dot google dot com>
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