From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17563 invoked by alias); 30 May 2006 08:20:47 -0000 Received: (qmail 17554 invoked by uid 22791); 30 May 2006 08:20:46 -0000 X-Spam-Check-By: sourceware.org Received: from ausmtp06.au.ibm.com (HELO ausmtp06.au.ibm.com) (202.81.18.155) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 30 May 2006 08:20:39 +0000 Received: from sd0208e0.au.ibm.com (d23rh904.au.ibm.com [202.81.18.202]) by ausmtp06.au.ibm.com (8.13.6/8.13.6) with ESMTP id k4U8MigR4972544 for ; Tue, 30 May 2006 18:22:44 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.250.237]) by sd0208e0.au.ibm.com (8.12.10/NCO/VER6.8) with ESMTP id k4U8NrrX193816 for ; Tue, 30 May 2006 18:23:54 +1000 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id k4U8KWRb014406 for ; Tue, 30 May 2006 18:20:32 +1000 Received: from [9.181.133.20] ([9.181.133.20]) by d23av04.au.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id k4U8KTNH014224 for ; Tue, 30 May 2006 18:20:31 +1000 Date: Tue, 30 May 2006 17:14:00 -0000 From: Wu Zhou To: gdb@sourceware.org Subject: A little patch for two comments in infrun.c Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2006-05/txt/msg00388.txt.bz2 Hi, I am reading the source of infrun.c, and having a couple questions about two comments in the code: First, in resume (int step, enum target_signal sig), one comment says: /* FIXME: calling breakpoint_here_p (read_pc ()) three times! */ Does this still make sense? In function resume, there does exist three call for breakpoint_here_p (read_pc ()). But read_pc () might return various values at various points. The breakpoint chain maintained in this function might also change as the execution proceeds. So I am thinking this comment doesn't make sense here. Am I right? Any error, feel free to correct me. I also think the comment in the following code (in handle_inferior_event) should be changed. /* If it's a new process, add it to the thread database */ ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid) && !ptid_equal (ecs->ptid, minus_one_ptid) && !in_thread_list (ecs->ptid)); IMHO, "new thread" describes more properly about the code. I have a patch for the above two comments. --- infrun.c.orig 2006-05-29 23:52:47.000000000 -0700 +++ infrun.c 2006-05-29 23:53:24.000000000 -0700 @@ -531,9 +531,6 @@ resume (int step, enum target_signal sig fprintf_unfiltered (gdb_stdlog, "infrun: resume (step=%d, signal=%d)\n", step, sig); - /* FIXME: calling breakpoint_here_p (read_pc ()) three times! */ - - /* Some targets (e.g. Solaris x86) have a kernel bug when stepping over an instruction that causes a page fault without triggering a hardware watchpoint. The kernel properly notices that it shouldn't @@ -1290,7 +1287,7 @@ handle_inferior_event (struct execution_ flush_cached_frames (); - /* If it's a new process, add it to the thread database */ + /* If it's a new thread, add it to the thread database */ ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid) && !ptid_equal (ecs->ptid, minus_one_ptid) OK to commit? Regards - Wu Zhou