This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Clarify infrun variable naming.
- From: Jim Blandy <jimb at codesourcery dot com>
- To: Vladimir Prus <vladimir at codesourcery dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Wed, 05 Dec 2007 12:59:26 -0800
- Subject: Re: [RFA] Clarify infrun variable naming.
- References: <200711231623.04823.vladimir@codesourcery.com> <m3ve7ej5xb.fsf@codesourcery.com> <200712051517.17764.vladimir@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;