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

[Pedro Alves <palves@redhat.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