From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25273 invoked by alias); 5 Dec 2007 20:59:37 -0000 Received: (qmail 25265 invoked by uid 22791); 5 Dec 2007 20:59:36 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 05 Dec 2007 20:59:30 +0000 Received: (qmail 15899 invoked from network); 5 Dec 2007 20:59:28 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 Dec 2007 20:59:28 -0000 To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Clarify infrun variable naming. References: <200711231623.04823.vladimir@codesourcery.com> <200712051517.17764.vladimir@codesourcery.com> From: Jim Blandy Date: Wed, 05 Dec 2007 21:32:00 -0000 In-Reply-To: <200712051517.17764.vladimir@codesourcery.com> (Vladimir Prus's message of "Wed, 5 Dec 2007 15:17:17 +0300") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-12/txt/msg00090.txt.bz2 Vladimir Prus writes: > On Wednesday 05 December 2007 04:17:52 Jim Blandy wrote: >> >> Vladimir Prus 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;