This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA/commit] Rename to_can_use_hw_breakpoint -> to_can_use_hw_watchpoint.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]