[RFA] Remove unecessary checks for macros in target.h

[RFA] Remove unecessary checks for macros in target.h

[Pierre Muller]

  target.h header has lots of

#ifndef A_MACRO
#define A_MACRO default_implementation
#endif 
constructs.
  
  This patch simply removes all these 
unnecessary checks for macros that are not defined anymore in any
config file, nor by configure script.
(Unless the configure scripts might set macros 
without having them inside their sources,
I checked all those macros by grep). 

  Is this OK?

Pierre Muller
Pascal language support maintainer for GDB

PS: STOPPED_BY_WATCHPOINT macro could probably also
loose its unused parameter...
Would this later patch be also welcome?


2009-05-08  Pierre Muller  <muller.u-strasbg.fr>

	* src/gdb/target.h: Remove all tests for already defined
	macros. All macros defined here should not be set in config
	headers anymore.

Index: src/gdb/target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.150
diff -u -p -r1.150 target.h
--- src/gdb/target.h	30 Apr 2009 02:55:27 -0000	1.150
+++ src/gdb/target.h	8 May 2009 05:49:10 -0000
@@ -1053,24 +1053,18 @@ extern char *normal_pid_to_str (ptid_t p
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory
read or
    write).  */
 
-#ifndef STOPPED_BY_WATCHPOINT
 #define STOPPED_BY_WATCHPOINT(w) \
    (*current_target.to_stopped_by_watchpoint) ()
-#endif
 
 /* Non-zero if we have steppable watchpoints  */
 
-#ifndef HAVE_STEPPABLE_WATCHPOINT
 #define HAVE_STEPPABLE_WATCHPOINT \
    (current_target.to_have_steppable_watchpoint)
-#endif
 
 /* Non-zero if we have continuable watchpoints  */
 
-#ifndef HAVE_CONTINUABLE_WATCHPOINT
 #define HAVE_CONTINUABLE_WATCHPOINT \
    (current_target.to_have_continuable_watchpoint)
-#endif
 
 /* Provide defaults for hardware watchpoint functions.  */
 
@@ -1082,41 +1076,31 @@ extern char *normal_pid_to_str (ptid_t p
    bp_hardware_breakpoint.  CNT is the number of such watchpoints used so
far
    (including this one?).  OTHERTYPE is who knows what...  */
 
-#ifndef TARGET_CAN_USE_HARDWARE_WATCHPOINT
 #define TARGET_CAN_USE_HARDWARE_WATCHPOINT(TYPE,CNT,OTHERTYPE) \
  (*current_target.to_can_use_hw_breakpoint) (TYPE, CNT, OTHERTYPE);
-#endif
 
-#ifndef TARGET_REGION_OK_FOR_HW_WATCHPOINT
 #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(addr, len) \
     (*current_target.to_region_ok_for_hw_watchpoint) (addr, len)
-#endif
 
 
 /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes.  TYPE
is 0
    for write, 1 for read, and 2 for read/write accesses.  Returns 0 for
    success, non-zero for failure.  */
 
-#ifndef target_insert_watchpoint
 #define	target_insert_watchpoint(addr, len, type)	\
      (*current_target.to_insert_watchpoint) (addr, len, type)
 
 #define	target_remove_watchpoint(addr, len, type)	\
      (*current_target.to_remove_watchpoint) (addr, len, type)
-#endif
 
-#ifndef target_insert_hw_breakpoint
 #define target_insert_hw_breakpoint(bp_tgt) \
      (*current_target.to_insert_hw_breakpoint) (bp_tgt)
 
 #define target_remove_hw_breakpoint(bp_tgt) \
      (*current_target.to_remove_hw_breakpoint) (bp_tgt)
-#endif
 
-#ifndef target_stopped_data_address
 #define target_stopped_data_address(target, x) \
     (*target.to_stopped_data_address) (target, x)
-#endif
 
 #define target_watchpoint_addr_within_range(target, addr, start, length) \
   (*target.to_watchpoint_addr_within_range) (target, addr, start, length)

Re: [RFA] Remove unecessary checks for macros in target.h

[Joel Brobecker]

> 2009-05-08  Pierre Muller  <muller.u-strasbg.fr>
> 
> 	* src/gdb/target.h: Remove all tests for already defined
> 	macros. All macros defined here should not be set in config
> 	headers anymore.

Looks good to me. Can you also remove the following comment that
now becomes obsolete?

    /* If the *_hw_beakpoint functions have not been defined
       elsewhere use the definitions in the target vector.  */

As a followup patch, I'd go one step further and rename the macros
that do not follow the general naming scheme in that file.
For instance: STOPPED_BY_WATCHPOINT -> target_stopped_by_watchpoint.

> PS: STOPPED_BY_WATCHPOINT macro could probably also loose its unused
> parameter...  Would this later patch be also welcome?

Sure. That could be taken care of as part of the renaming above.

-- 
Joel

Re: [RFA] Remove unecessary checks for macros in target.h

[David Daney]

Pierre Muller wrote:
>   target.h header has lots of
> 
> #ifndef A_MACRO
> #define A_MACRO default_implementation
> #endif 
> constructs.
>   
>   This patch simply removes all these 
> unnecessary checks for macros that are not defined anymore in any
> config file, nor by configure script.
> (Unless the configure scripts might set macros 
> without having them inside their sources,
> I checked all those macros by grep). 
> 
>   Is this OK?
> 

I don't know if it is OK.

However, I do wonder if it would make sense to remove the macros 
altogether, and just push the expansions down into the  code.  If we 
really don't want to be able to switch in different implementations of 
these things, what is the point of an added layer of abstraction?

David Daney

Re: [RFA] Remove unecessary checks for macros in target.h

[Joel Brobecker]

> However, I do wonder if it would make sense to remove the macros  
> altogether, and just push the expansions down into the  code.  If we  
> really don't want to be able to switch in different implementations of  
> these things, what is the point of an added layer of abstraction?

Eventually, we'll want these macros to be functions, I think.
I'd personally like it if they all had the target_ops as the first
parameter, so that the target function can find the "target beneath"
and call its associated routine if needed. For instance:

  static void
  sol_thread_detach (struct target_ops *ops, char *args, int from_tty)
  {
    struct target_ops *beneath = find_target_beneath (ops);
  
    sol_thread_active = 0;
    inferior_ptid = pid_to_ptid (PIDGET (main_ph.ptid));
    unpush_target (ops);
    beneath->to_detach (beneath, args, from_tty);
  }

-- 
Joel