From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3235 invoked by alias); 8 Sep 2007 11:07:17 -0000 Received: (qmail 3154 invoked by uid 22791); 8 Sep 2007 11:07:16 -0000 X-Spam-Check-By: sourceware.org Received: from romy.inter.net.il (HELO romy.inter.net.il) (213.8.233.24) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 08 Sep 2007 11:07:11 +0000 Received: from HOME-C4E4A596F7 (IGLD-80-230-141-251.inter.net.il [80.230.141.251]) by romy.inter.net.il (MOS 3.7.3-GA) with ESMTP id IVN33225 (AUTH halo1); Sat, 8 Sep 2007 14:06:59 +0300 (IDT) Date: Sat, 08 Sep 2007 11:07:00 -0000 Message-Id: From: Eli Zaretskii To: Vladimir Prus CC: gdb-patches@sources.redhat.com In-reply-to: <200709080018.25052.vladimir@codesourcery.com> (message from Vladimir Prus on Sat, 8 Sep 2007 00:18:24 +0400) Subject: Re: [4/9] associate bpstat with location Reply-to: Eli Zaretskii References: <200709080018.25052.vladimir@codesourcery.com> 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-09/txt/msg00105.txt.bz2 > From: Vladimir Prus > Date: Sat, 8 Sep 2007 00:18:24 +0400 > > (bpstat_find_breakpoint): Look at bpstat's location's > owner, not at bpstat->breakpoint_at. > (bpstat_find_step_resume_breakpoint): Likewise. > (bpstat_num): Likewise. > (print_it_typical): Likewise. > (print_bp_stop_message): Likewise. > (watchpoint_check): Likewise. > (bpstat_what): Likewise. > (bpstat_get_triggered_catchpoints): Likewise. > (breakpoint_auto_delete): Likewise. > (delete_breakpoint): Likewise. A minor stylistic point: could we please avoid the annoying "Likewise"s? The canonical way of writing a ChangeLog entry for several functions with an identical change is this: (bpstat_find_breakpoint, bpstat_find_step_resume_breakpoint) (bpstat_num, print_it_typical): Look at bpstat's location's owner, not at bpstat->breakpoint_at. I'm quite sure the GNU coding standards describe this. (Yes, I know that our ChangeLog's abuse "Likewise" too much.) > - b = (*bsp)->breakpoint_at; > + /* We assume we'll never have several bpstats that > + correspond to a single breakpoint -- otherwise, > + this function might return the same number more > + than once and this will look ugly. */ > + b = (*bsp)->breakpoint_at ? (*bsp)->breakpoint_at->owner : NULL; Is the assumption in the comment above really valid? I happen to put several breakpoints on the same line quite a lot (each breakpoint has a different condition and/or different commands list). Can we do better, even if it requires to try harder? > case bp_access_watchpoint: > if (bs->old_val != NULL) > { > - annotate_watchpoint (bs->breakpoint_at->number); > + annotate_watchpoint (b->number); Watchpoints also? Did you make corresponding changes in the code that sets watchpoints? Thanks.