Re: [RFA] Clarify infrun variable naming.

[Jim Blandy <jimb@codesourcery.com>]

Vladimir Prus <vladimir at codesourcery.com> writes:
> On Wednesday 05 December 2007 04:17:52 Jim Blandy wrote:
>> 
>> Vladimir Prus <vladimir at codesourcery.com> writes:
>> > The infrun.c file has a variable named trap_expected, which
>> > is a bit misleading -- after all, most times when we resume
>> > inferior, we get SIGTRAP. As it turns out, that variable
>> > is set when we're stepping over breakpoints, so a better
>> > name would be stepping_over_breakpoint. Likewise,
>> > ecs->another_trap also indicates that keep_going should
>> > be stepping over breakpoint. The attached patch clarifies
>> > the naming and adds comments, and has no behaviour changes.
>> > (The patch is on top of my previous breakpoints_inserted
>> > removing patch).
>> > OK?
> ...
>> So, my suggestions were:
>> 
>> - We should replace stepping_past_breakpoint and
>>   stepping_past_breakpoint_ptid with a single ptid_t variable,
>>   deferred_step_ptid.
>> 
>> - We should rename trap_expected to stepped_over_breakpoint.  The past
>>   tense 'stepped' suggests that we're talking about a step which has
>>   already happened (which is true by the time we reach
>>   handle_inferior_event).
>> 
>> (And if I meditate carefully enough on the best possible names, it'll
>> be quite some time before I have to look at Vlad's harder patches.  :))
>
> Here's the patch that renamed stepping_past_breakpoint_ptid. I attach both
> patch and the delta relative to the previous revision.
>
> The 'stepped' vs. 'stepping' change was not done -- for reason I've posted
> already.

Sure, that's fine.  I agree with your and Dan's arguments.

I noticed some text issues; with those fixed, this can go in.

> +/* Nonzero if we are presenting stepping over breakpoint.

This should be "over a breakpoint".

> -static int trap_expected;
> +   If we hit a breakpoint or watchpoint, and then continue,
> +   we need to single step the current thread with breakpoints
> +   disabled, so that to avoid hitting the same breakpoint or

"so that" needs to be deleted, or perhaps changed to "so as"?

> +   watchpoint again.  And we should step just a single
> +   thread and keep other threads stopped, so that 
> +   other threads don't miss breakpoints while they are removed.
> +
> +   So, this variable simultaneously means that we need to single
> +   step current thread, keep other threads stopped, and that
> +   breakpoints should be removed while we step.

"step the current thread".

> +   This variable is set either:
> +   - in proceed, when we resume inferior on explicit user's request

"resume the inferior at the user's explicit request"

> +/* If not equal to null_ptid, means that after stepping over breakpoint

'this means'

> +   is finished, we need to switch to deferred_step_ptid, and step it.
> +
> +   The use case is when a breakpoint in one thread, and then the user 

'when one thread has hit a breakpoint'?

> +   has switched to another thread and issued 'step'. We need to step over
> +   breakpoint in the thread which hit breakpoint, but then continue

'the breakpoint'

> +   stepping the thread user has selected.  */
> +static ptid_t deferred_step_ptid;