From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7158 invoked by alias); 6 Oct 2004 16:09:59 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 7043 invoked from network); 6 Oct 2004 16:09:53 -0000 Received: from unknown (HELO legolas.inter.net.il) (192.114.186.24) by sourceware.org with SMTP; 6 Oct 2004 16:09:53 -0000 Received: from zaretski ([80.230.143.237]) by legolas.inter.net.il (MOS 3.5.3-GR) with ESMTP id CTM21310 (AUTH halo1); Wed, 6 Oct 2004 18:09:19 +0200 (IST) Date: Wed, 06 Oct 2004 16:09:00 -0000 From: "Eli Zaretskii" To: Jeff Johnston Message-ID: <01c4abbe$Blat.v2.2.2$89197760@zahav.net.il> Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=ISO-8859-1 CC: gdb-patches@sources.redhat.com In-reply-to: <4154A2B1.9010908@redhat.com> (message from Jeff Johnston on Fri, 24 Sep 2004 18:41:53 -0400) Subject: Re: [RFA]: Change to_stopped_data_address ABI Reply-to: Eli Zaretskii References: <4134C991.7050507@redhat.com> <414F4D15.7010503@redhat.com> <01c49f8f$Blat.v2.2.2$a6da98a0@zahav.net.il> <4154A2B1.9010908@redhat.com> X-SW-Source: 2004-10/txt/msg00116.txt.bz2 > Date: Fri, 24 Sep 2004 18:41:53 -0400 > From: Jeff Johnston > Cc: gdb-patches@sources.redhat.com Sorry for a late response. > Andrew/Eli, ok to commit? Did I miss anything? Thanks, most of the concerns I had with your original patch are now gone. Still, a few minor ones remain, see below. > +int > +frv_stopped_data_address (CORE_ADDR *addr_p) > { > CORE_ADDR brr, dbar0, dbar1, dbar2, dbar3; > > @@ -1305,15 +1305,27 @@ frv_stopped_data_address (void) > dbar3 = read_register (dbar3_regnum); > > if (brr & (1<<11)) > - return dbar0; > + *addr_p = dbar0; > else if (brr & (1<<10)) > - return dbar1; > + *addr_p = dbar1; > else if (brr & (1<<9)) > - return dbar2; > + *addr_p = dbar2; > else if (brr & (1<<8)) > - return dbar3; > + *addr_p = dbar3; > else > - return 0; > + { > + *addr_p = 0; > + return 0; > + } > + > + return 1; > +} I don't understand why do you put a zero into the address pointed by addr_p in the case that no watchpoint has fired. It is customary to leave the arguments unaltered in such cases. isn't it enough that you return a zero as the function's value? Similar code is in the other functions that return the stopped data address; I have similar issue with them. > RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v > retrieving revision 1.6 > diff -u -p -r1.6 nm-i386.h > --- config/i386/nm-i386.h 13 Sep 2004 14:06:03 -0000 1.6 > +++ config/i386/nm-i386.h 24 Sep 2004 22:34:27 -0000 > @@ -47,10 +47,10 @@ extern int i386_region_ok_for_watchpoint > triggered. */ > extern int i386_stopped_by_hwbp (void); > > -/* If the inferior has some break/watchpoint that triggered, return > +/* If the inferior has some break/watchpoint that triggered, set > the address associated with that break/watchpoint. Otherwise, > - return zero. */ > -extern CORE_ADDR i386_stopped_data_address (void); > + set the watchpoint address to zero. Always return true. */ The last sentence is not true: we return zero (false) sometimes. > +@findex i386_stopped_by_watchpoint > +@item i386_stopped_by_watchpoint (void) > +The macro @code{STOPPED_BY_WATCHPOINT} > +is set to call this function. The > +argument passed to @code{STOPPED_BY_WATCHPOINT} is ignored. This > +function uses the same logic as @code{i386_stopped_data_address}. I'd prefer if we describe the operation of i386_stopped_by_watchpoint explicitly, not by a reference to the logic of i386_stopped_data_address.