From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30606 invoked by alias); 13 Jan 2010 14:07:15 -0000 Received: (qmail 30577 invoked by uid 22791); 13 Jan 2010 14:07:13 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 13 Jan 2010 14:07:08 +0000 Received: (qmail 6933 invoked from network); 13 Jan 2010 14:07:05 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 13 Jan 2010 14:07:05 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFA/commit] Rename to_can_use_hw_breakpoint -> to_can_use_hw_watchpoint. Date: Wed, 13 Jan 2010 14:07:00 -0000 User-Agent: KMail/1.9.10 Cc: Joel Brobecker References: <1263380049-6804-1-git-send-email-brobecker@adacore.com> In-Reply-To: <1263380049-6804-1-git-send-email-brobecker@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <201001131407.04501.pedro@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: 2010-01/txt/msg00342.txt.bz2 On Wednesday 13 January 2010 10:54:09, Joel Brobecker wrote: > I noticed this while reviewing the patches sent by Luis and Thiago. > The target_ops to_can_use_hw_breakpoint routine is really meant to > check for H/W *watchpoint* resources, AFAICT. That's not actually true. It is actually used to check for both harware breapoints and watchpoints resources, depending on the `type' argument. See create_breakpoint or do_enable_breakpoint where it is called for hw breakpoints. See also the i386 support or nto-procfs.c:procfs_hw_watchpoint for examples showing that hw breakpoints and watchpoints tend to be two faces of the same coin. Of course, this also shows that hw_breakpoint_used_count and hw_watchpoint_used_count are completely busted by design, for not consulting the target properly. Renaming the target method/macro instead would be fine too, as "breakpoint" sounds a bit more generic than "watchpoint" (to me). Out of curiosity, the similar situation in gdbserver lead to that insert_point/remove_point naming. > In fact, the macro wrapping the call to this routine in current_target > is actually named target_can_use_hardware_watchpoint. This patch renames > this routine in the target_ops vector, as well as all > target-specific implementations, to keep things consistent. Yeah. No real objections from me. Consistency is good. > The update was performed mechanically using perl -pi -e one-liners. > > gdb/ChangeLog: > > Rename to_can_use_hw_breakpoint -> to_can_use_hw_watchpoint. > * target.h (struct target_ops): Rename to_can_use_hw_breakpoint > into to_can_use_hw_watchpoint. > (target_can_use_hardware_watchpoint): Adjust accordingly. > * target.c (debug_to_can_use_hw_watchpoint): Renames > debug_to_can_use_hw_breakpoint. Adjust following the rename > in struct target_ops. > (update_current_target, setup_target_debug): Adjust following > the rename in struct target_ops. > > Adjust the rest of the code accordingly: > * i386-nat.c, ia64-linux-nat.c, inf-ttrace.c, mips-linux-nat.c, > mips-linux-nat.c, nto-procfs.c, ppc-linux-nat.c, procfs.c, > remote-m32r-sdi.c, remote-mips.c, remote.c, s390-nat.c, > spu-linux-nat.c > > Make the following renames for consistency, and adjust the rest > of the code in the same file accordingly: > * i386-nat.c (i386_can_use_hw_watchpoint): Renames > i386_can_use_hw_breakpoint. > * ia64-linux-nat.c (ia64_linux_can_use_hw_watchpoint): Renames > ia64_linux_can_use_hw_breakpoint. > accordingly. > * inf-ttrace.c (inf_ttrace_can_use_hw_watchpoint): Renames > inf_ttrace_can_use_hw_breakpoint. > accordingly. > * mips-linux-nat.c (mips_linux_can_use_hw_watchpoint): Renames > mips_linux_can_use_hw_breakpoint. > * nto-procfs.c (procfs_can_use_hw_watchpoint): Renames > procfs_can_use_hw_breakpoint. > * nto-procfs.c (procfs_can_use_hw_watchpoint): Renames > procfs_can_use_hw_breakpoint. > * s390-nat.c (s390_can_use_hw_watchpoint): Renames > s390_can_use_hw_breakpoint. > * spu-linux-nat.c (spu_can_use_hw_watchpoint): Renames > spu_can_use_hw_breakpoint. > > Tested on x86_64-linux. Any objections? > > --- > gdb/i386-nat.c | 4 ++-- > gdb/ia64-linux-nat.c | 4 ++-- > gdb/inf-ttrace.c | 4 ++-- > gdb/mips-linux-nat.c | 6 +++--- > gdb/nto-procfs.c | 6 +++--- > gdb/ppc-linux-nat.c | 2 +- > gdb/procfs.c | 8 ++++---- > gdb/remote-m32r-sdi.c | 2 +- > gdb/remote-mips.c | 2 +- > gdb/remote.c | 2 +- > gdb/s390-nat.c | 4 ++-- > gdb/spu-linux-nat.c | 6 +++--- > gdb/target.c | 14 +++++++------- > gdb/target.h | 4 ++-- > 14 files changed, 34 insertions(+), 34 deletions(-) > > diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c > index fa0cce6..2d12a3a 100644 > --- a/gdb/i386-nat.c > +++ b/gdb/i386-nat.c > @@ -639,7 +639,7 @@ i386_remove_hw_breakpoint (struct gdbarch *gdbarch, > sharing implemented via reference counts in i386-nat.c. */ > > static int > -i386_can_use_hw_breakpoint (int type, int cnt, int othertype) > +i386_can_use_hw_watchpoint (int type, int cnt, int othertype) > { > return 1; > } > @@ -673,7 +673,7 @@ i386_use_watchpoints (struct target_ops *t) > But we do need to reset the status register to avoid another trap. */ > t->to_have_continuable_watchpoint = 1; > > - t->to_can_use_hw_breakpoint = i386_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = i386_can_use_hw_watchpoint; > t->to_region_ok_for_hw_watchpoint = i386_region_ok_for_watchpoint; > t->to_stopped_by_watchpoint = i386_stopped_by_watchpoint; > t->to_stopped_data_address = i386_stopped_data_address; > diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c > index e6a7077..e4e9647 100644 > --- a/gdb/ia64-linux-nat.c > +++ b/gdb/ia64-linux-nat.c > @@ -663,7 +663,7 @@ ia64_linux_stopped_by_watchpoint (void) > } > > static int > -ia64_linux_can_use_hw_breakpoint (int type, int cnt, int othertype) > +ia64_linux_can_use_hw_watchpoint (int type, int cnt, int othertype) > { > return 1; > } > @@ -837,7 +837,7 @@ _initialize_ia64_linux_nat (void) > without triggering a watchpoint. */ > > t->to_have_steppable_watchpoint = 1; > - t->to_can_use_hw_breakpoint = ia64_linux_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = ia64_linux_can_use_hw_watchpoint; > t->to_stopped_by_watchpoint = ia64_linux_stopped_by_watchpoint; > t->to_stopped_data_address = ia64_linux_stopped_data_address; > t->to_insert_watchpoint = ia64_linux_insert_watchpoint; > diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c > index c9ab548..b3dcf17 100644 > --- a/gdb/inf-ttrace.c > +++ b/gdb/inf-ttrace.c > @@ -357,7 +357,7 @@ inf_ttrace_remove_watchpoint (CORE_ADDR addr, int len, int type) > } > > static int > -inf_ttrace_can_use_hw_breakpoint (int type, int len, int ot) > +inf_ttrace_can_use_hw_watchpoint (int type, int len, int ot) > { > return (type == bp_hardware_watchpoint); > } > @@ -1260,7 +1260,7 @@ inf_ttrace_target (void) > t->to_resume = inf_ttrace_resume; > t->to_wait = inf_ttrace_wait; > t->to_files_info = inf_ttrace_files_info; > - t->to_can_use_hw_breakpoint = inf_ttrace_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = inf_ttrace_can_use_hw_watchpoint; > t->to_insert_watchpoint = inf_ttrace_insert_watchpoint; > t->to_remove_watchpoint = inf_ttrace_remove_watchpoint; > t->to_stopped_by_watchpoint = inf_ttrace_stopped_by_watchpoint; > diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c > index fe05192..739ed07 100644 > --- a/gdb/mips-linux-nat.c > +++ b/gdb/mips-linux-nat.c > @@ -672,11 +672,11 @@ type_to_irw (int type) > } > } > > -/* Target to_can_use_hw_breakpoint implementation. Return 1 if we can > +/* Target to_can_use_hw_watchpoint implementation. Return 1 if we can > handle the specified watch type. */ > > static int > -mips_linux_can_use_hw_breakpoint (int type, int cnt, int ot) > +mips_linux_can_use_hw_watchpoint (int type, int cnt, int ot) > { > int i; > uint32_t wanted_mask, irw_mask; > @@ -1071,7 +1071,7 @@ triggers a breakpoint or watchpoint."), > t->to_fetch_registers = mips64_linux_fetch_registers; > t->to_store_registers = mips64_linux_store_registers; > > - t->to_can_use_hw_breakpoint = mips_linux_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = mips_linux_can_use_hw_watchpoint; > t->to_remove_watchpoint = mips_linux_remove_watchpoint; > t->to_insert_watchpoint = mips_linux_insert_watchpoint; > t->to_stopped_by_watchpoint = mips_linux_stopped_by_watchpoint; > diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c > index d8f3c91..9b4c8d1 100644 > --- a/gdb/nto-procfs.c > +++ b/gdb/nto-procfs.c > @@ -70,7 +70,7 @@ static void init_procfs_ops (void); > > static ptid_t do_attach (ptid_t ptid); > > -static int procfs_can_use_hw_breakpoint (int, int, int); > +static int procfs_can_use_hw_watchpoint (int, int, int); > > static int procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type); > > @@ -1411,7 +1411,7 @@ init_procfs_ops (void) > procfs_ops.to_files_info = procfs_files_info; > procfs_ops.to_insert_breakpoint = procfs_insert_breakpoint; > procfs_ops.to_remove_breakpoint = procfs_remove_breakpoint; > - procfs_ops.to_can_use_hw_breakpoint = procfs_can_use_hw_breakpoint; > + procfs_ops.to_can_use_hw_watchpoint = procfs_can_use_hw_watchpoint; > procfs_ops.to_insert_hw_breakpoint = procfs_insert_hw_breakpoint; > procfs_ops.to_remove_hw_breakpoint = procfs_remove_breakpoint; > procfs_ops.to_insert_watchpoint = procfs_insert_hw_watchpoint; > @@ -1503,7 +1503,7 @@ procfs_hw_watchpoint (int addr, int len, int type) > } > > static int > -procfs_can_use_hw_breakpoint (int type, int cnt, int othertype) > +procfs_can_use_hw_watchpoint (int type, int cnt, int othertype) > { > return 1; > } > diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c > index 10ff73d..4c9ed2d 100644 > --- a/gdb/ppc-linux-nat.c > +++ b/gdb/ppc-linux-nat.c > @@ -1644,7 +1644,7 @@ _initialize_ppc_linux_nat (void) > t->to_store_registers = ppc_linux_store_inferior_registers; > > /* Add our watchpoint methods. */ > - t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources; > + t->to_can_use_hw_watchpoint = ppc_linux_check_watch_resources; > t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint; > t->to_insert_watchpoint = ppc_linux_insert_watchpoint; > t->to_remove_watchpoint = ppc_linux_remove_watchpoint; > diff --git a/gdb/procfs.c b/gdb/procfs.c > index 9278bcb..ae1cf63 100644 > --- a/gdb/procfs.c > +++ b/gdb/procfs.c > @@ -153,7 +153,7 @@ static int proc_find_memory_regions (int (*) (CORE_ADDR, > > static char * procfs_make_note_section (bfd *, int *); > > -static int procfs_can_use_hw_breakpoint (int, int, int); > +static int procfs_can_use_hw_watchpoint (int, int, int); > > #if defined (PR_MODEL_NATIVE) && (PR_MODEL_NATIVE == PR_MODEL_LP64) > /* When GDB is built as 64-bit application on Solaris, the auxv data is > @@ -5316,12 +5316,12 @@ procfs_set_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rwflag, > or bp_hardware_watchpoint. CNT is the number of watchpoints used so > far. > > - Note: procfs_can_use_hw_breakpoint() is not yet used by all > + Note: procfs_can_use_hw_watchpoint() is not yet used by all > procfs.c targets due to the fact that some of them still define > target_can_use_hardware_watchpoint. */ > > static int > -procfs_can_use_hw_breakpoint (int type, int cnt, int othertype) > +procfs_can_use_hw_watchpoint (int type, int cnt, int othertype) > { > /* Due to the way that proc_set_watchpoint() is implemented, host > and target pointers must be of the same size. If they are not, > @@ -5419,7 +5419,7 @@ procfs_use_watchpoints (struct target_ops *t) > t->to_insert_watchpoint = procfs_insert_watchpoint; > t->to_remove_watchpoint = procfs_remove_watchpoint; > t->to_region_ok_for_hw_watchpoint = procfs_region_ok_for_hw_watchpoint; > - t->to_can_use_hw_breakpoint = procfs_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = procfs_can_use_hw_watchpoint; > } > > /* > diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c > index be6a564..c02954b 100644 > --- a/gdb/remote-m32r-sdi.c > +++ b/gdb/remote-m32r-sdi.c > @@ -1622,7 +1622,7 @@ init_m32r_ops (void) > m32r_ops.to_files_info = m32r_files_info; > m32r_ops.to_insert_breakpoint = m32r_insert_breakpoint; > m32r_ops.to_remove_breakpoint = m32r_remove_breakpoint; > - m32r_ops.to_can_use_hw_breakpoint = m32r_can_use_hw_watchpoint; > + m32r_ops.to_can_use_hw_watchpoint = m32r_can_use_hw_watchpoint; > m32r_ops.to_insert_watchpoint = m32r_insert_watchpoint; > m32r_ops.to_remove_watchpoint = m32r_remove_watchpoint; > m32r_ops.to_stopped_by_watchpoint = m32r_stopped_by_watchpoint; > diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c > index f2fb8f3..2b1b02b 100644 > --- a/gdb/remote-mips.c > +++ b/gdb/remote-mips.c > @@ -3346,7 +3346,7 @@ _initialize_remote_mips (void) > mips_ops.to_insert_watchpoint = mips_insert_watchpoint; > mips_ops.to_remove_watchpoint = mips_remove_watchpoint; > mips_ops.to_stopped_by_watchpoint = mips_stopped_by_watchpoint; > - mips_ops.to_can_use_hw_breakpoint = mips_can_use_watchpoint; > + mips_ops.to_can_use_hw_watchpoint = mips_can_use_watchpoint; > mips_ops.to_kill = mips_kill; > mips_ops.to_load = mips_load; > mips_ops.to_create_inferior = mips_create_inferior; > diff --git a/gdb/remote.c b/gdb/remote.c > index 9c50f7e..2beaecc 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -9348,7 +9348,7 @@ Specify the serial device it is connected to\n\ > remote_ops.to_remove_breakpoint = remote_remove_breakpoint; > remote_ops.to_stopped_by_watchpoint = remote_stopped_by_watchpoint; > remote_ops.to_stopped_data_address = remote_stopped_data_address; > - remote_ops.to_can_use_hw_breakpoint = remote_check_watch_resources; > + remote_ops.to_can_use_hw_watchpoint = remote_check_watch_resources; > remote_ops.to_insert_hw_breakpoint = remote_insert_hw_breakpoint; > remote_ops.to_remove_hw_breakpoint = remote_remove_hw_breakpoint; > remote_ops.to_insert_watchpoint = remote_insert_watchpoint; > diff --git a/gdb/s390-nat.c b/gdb/s390-nat.c > index 3af42ff..bb7e662 100644 > --- a/gdb/s390-nat.c > +++ b/gdb/s390-nat.c > @@ -384,7 +384,7 @@ s390_remove_watchpoint (CORE_ADDR addr, int len, int type) > } > > static int > -s390_can_use_hw_breakpoint (int type, int cnt, int othertype) > +s390_can_use_hw_watchpoint (int type, int cnt, int othertype) > { > return type == bp_hardware_watchpoint; > } > @@ -488,7 +488,7 @@ _initialize_s390_nat (void) > t->to_store_registers = s390_linux_store_inferior_registers; > > /* Add our watchpoint methods. */ > - t->to_can_use_hw_breakpoint = s390_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = s390_can_use_hw_watchpoint; > t->to_region_ok_for_hw_watchpoint = s390_region_ok_for_hw_watchpoint; > t->to_have_continuable_watchpoint = 1; > t->to_stopped_by_watchpoint = s390_stopped_by_watchpoint; > diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c > index 7c9e2c5..ca8d5f1 100644 > --- a/gdb/spu-linux-nat.c > +++ b/gdb/spu-linux-nat.c > @@ -576,9 +576,9 @@ spu_xfer_partial (struct target_ops *ops, > return -1; > } > > -/* Override the to_can_use_hw_breakpoint routine. */ > +/* Override the to_can_use_hw_watchpoint routine. */ > static int > -spu_can_use_hw_breakpoint (int type, int cnt, int othertype) > +spu_can_use_hw_watchpoint (int type, int cnt, int othertype) > { > return 0; > } > @@ -599,7 +599,7 @@ _initialize_spu_nat (void) > t->to_fetch_registers = spu_fetch_inferior_registers; > t->to_store_registers = spu_store_inferior_registers; > t->to_xfer_partial = spu_xfer_partial; > - t->to_can_use_hw_breakpoint = spu_can_use_hw_breakpoint; > + t->to_can_use_hw_watchpoint = spu_can_use_hw_watchpoint; > > /* Register SPU target. */ > add_target (t); > diff --git a/gdb/target.c b/gdb/target.c > index 25a2cd7..a78e770 100644 > --- a/gdb/target.c > +++ b/gdb/target.c > @@ -112,7 +112,7 @@ static int debug_to_insert_breakpoint (struct gdbarch *, > static int debug_to_remove_breakpoint (struct gdbarch *, > struct bp_target_info *); > > -static int debug_to_can_use_hw_breakpoint (int, int, int); > +static int debug_to_can_use_hw_watchpoint (int, int, int); > > static int debug_to_insert_hw_breakpoint (struct gdbarch *, > struct bp_target_info *); > @@ -619,7 +619,7 @@ update_current_target (void) > INHERIT (to_files_info, t); > INHERIT (to_insert_breakpoint, t); > INHERIT (to_remove_breakpoint, t); > - INHERIT (to_can_use_hw_breakpoint, t); > + INHERIT (to_can_use_hw_watchpoint, t); > INHERIT (to_insert_hw_breakpoint, t); > INHERIT (to_remove_hw_breakpoint, t); > INHERIT (to_insert_watchpoint, t); > @@ -732,7 +732,7 @@ update_current_target (void) > memory_insert_breakpoint); > de_fault (to_remove_breakpoint, > memory_remove_breakpoint); > - de_fault (to_can_use_hw_breakpoint, > + de_fault (to_can_use_hw_watchpoint, > (int (*) (int, int, int)) > return_zero); > de_fault (to_insert_hw_breakpoint, > @@ -3152,14 +3152,14 @@ debug_to_remove_breakpoint (struct gdbarch *gdbarch, > } > > static int > -debug_to_can_use_hw_breakpoint (int type, int cnt, int from_tty) > +debug_to_can_use_hw_watchpoint (int type, int cnt, int from_tty) > { > int retval; > > - retval = debug_target.to_can_use_hw_breakpoint (type, cnt, from_tty); > + retval = debug_target.to_can_use_hw_watchpoint (type, cnt, from_tty); > > fprintf_unfiltered (gdb_stdlog, > - "target_can_use_hw_breakpoint (%ld, %ld, %ld) = %ld\n", > + "target_can_use_hw_watchpoint (%ld, %ld, %ld) = %ld\n", > (unsigned long) type, > (unsigned long) cnt, > (unsigned long) from_tty, > @@ -3524,7 +3524,7 @@ setup_target_debug (void) > current_target.to_files_info = debug_to_files_info; > current_target.to_insert_breakpoint = debug_to_insert_breakpoint; > current_target.to_remove_breakpoint = debug_to_remove_breakpoint; > - current_target.to_can_use_hw_breakpoint = debug_to_can_use_hw_breakpoint; > + current_target.to_can_use_hw_watchpoint = debug_to_can_use_hw_watchpoint; > current_target.to_insert_hw_breakpoint = debug_to_insert_hw_breakpoint; > current_target.to_remove_hw_breakpoint = debug_to_remove_hw_breakpoint; > current_target.to_insert_watchpoint = debug_to_insert_watchpoint; > diff --git a/gdb/target.h b/gdb/target.h > index 20cbe29..c92fc14 100644 > --- a/gdb/target.h > +++ b/gdb/target.h > @@ -412,7 +412,7 @@ struct target_ops > void (*to_files_info) (struct target_ops *); > int (*to_insert_breakpoint) (struct gdbarch *, struct bp_target_info *); > int (*to_remove_breakpoint) (struct gdbarch *, struct bp_target_info *); > - int (*to_can_use_hw_breakpoint) (int, int, int); > + int (*to_can_use_hw_watchpoint) (int, int, int); > int (*to_insert_hw_breakpoint) (struct gdbarch *, struct bp_target_info *); > int (*to_remove_hw_breakpoint) (struct gdbarch *, struct bp_target_info *); > int (*to_remove_watchpoint) (CORE_ADDR, int, int); > @@ -1236,7 +1236,7 @@ extern char *normal_pid_to_str (ptid_t ptid); > (including this one?). OTHERTYPE is who knows what... */ > > #define target_can_use_hardware_watchpoint(TYPE,CNT,OTHERTYPE) \ > - (*current_target.to_can_use_hw_breakpoint) (TYPE, CNT, OTHERTYPE); > + (*current_target.to_can_use_hw_watchpoint) (TYPE, CNT, OTHERTYPE); > > #define target_region_ok_for_hw_watchpoint(addr, len) \ > (*current_target.to_region_ok_for_hw_watchpoint) (addr, len) -- Pedro Alves