Re: [RFC] Remove i386 low level debug register function from nm- header file.

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Pierre Muller wrote:

> Following my first RFC
> http://sourceware.org/ml/gdb-patches/2009-03/msg00197.html
> 
> and the following answers, I resubmit
> a new version where all i386 low level debug register
> is contained in a single struct defined in the 
> new i386-nat.h header (to comply with DOS file name restrictions
> as asked for by Eli).

This looks good to me.  However, now that you've moved the
prototypes to i386-nat.h, there is no longer any need for
the config/i386/nm-i386.h file -- please get rid of that file
(by removing the places where it is included into other nm-
files).

Also, it seems to me that now there is no need any more for
the I386_USE_GENERIC_WATCHPOINTS define -- targets that use
watchpoints call the i386_use_watchpoints routines, those
that don't, do not.

I think you should provide everything in i386-nat.c (and
the new i386-nat.h) unconditionally, and eliminate the
various definitions of I386_USE_GENERIC_WATCHPOINTS in the
nm- header files.

Note that in combination with removing nm-i386.h, this 
actually makes several nm- files completely empty, so they
should be removed as well.

> 2009-05-10  Pierre Muller  <muller.u-strasbg.fr>
> 
> 	Remove all i386 debug register low level macros in config tm files.

Should read "nm" files, not tm.

> +/* Targets should define this to use the generic x86 watchpoint support.
> */
> +#ifdef I386_USE_GENERIC_WATCHPOINTS

As mentioned above, please get rid of this conditional ...

> +
> +/* Add watchpoint methods to the provided target_ops.  
> +   Targets defining I386_USE_GENERIC_WATCHPOINTS should call
> +   this.  */

... as well as the second sentence of the comment.

>  /* Support for 8-byte wide hw watchpoints.  */
>  #ifndef TARGET_HAS_DR_LEN_8
> -#define TARGET_HAS_DR_LEN_8	0
> +#define TARGET_HAS_DR_LEN_8 (i386_dr_low.debug_register_length == 8)
>  #endif

Nobody should be defining this any more, so please eliminate
the #ifndef ... conditional. 

> Index: src/gdb/i386fbsd-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386fbsd-nat.c,v
> retrieving revision 1.18
> @@ -126,7 +127,19 @@ _initialize_i386fbsd_nat (void)
>  
>    /* Add some extra features to the common *BSD/i386 target.  */
>    t = i386bsd_target ();
> +
> +#ifdef I386_USE_GENERIC_WATCHPOINTS
> +
>    i386_use_watchpoints (t);
> +
> +  i386_dr_low.set_control = i386bsd_dr_set_control;
> +  i386_dr_low.set_addr = i386bsd_dr_set_addr;
> +  i386_dr_low.reset_addr = i386bsd_dr_reset_addr;
> +  i386_dr_low.get_status = i386bsd_dr_get_status;
> +  i386_set_debug_register_length (4);
> +#endif /* I386_USE_GENERIC_WATCHPOINTS */


In this case, because the header file currently conditionally defines
I386_USE_GENERIC_WATCHPOINTS:

#ifdef HAVE_PT_GETDBREGS
#define I386_USE_GENERIC_WATCHPOINTS
#endif

you should move that condition to the i386fbsd-nat.c file:

#ifdef HAVE_PT_GETDBREGS
   i386_use_watchpoints (t);

   ...
#endif

> diff -u -p -r1.189 windows-nat.c
> --- windows-nat.c	17 Apr 2009 15:44:28 -0000	1.189
> +++ windows-nat.c	8 May 2009 14:02:25 -0000
> @@ -2166,8 +2171,22 @@ init_windows_ops (void)
>    windows_ops.to_has_execution = 1;
>    windows_ops.to_pid_to_exec_file = windows_pid_to_exec_file;
>    windows_ops.to_get_ada_task_ptid = windows_get_ada_task_ptid;
> +
> +#ifdef I386_USE_GENERIC_WATCHPOINTS
> +
>    i386_use_watchpoints (&windows_ops);
>  
> +  i386_dr_low.set_control = cygwin_set_dr7;
> +  i386_dr_low.set_addr = cygwin_set_dr;
> +  i386_dr_low.reset_addr = NULL;
> +  i386_dr_low.get_status = cygwin_get_dr6;

Here, it seems every user of windows-nat.c unconditionally
defines I386_USE_GENERIC_WATCHPOINTS, so there's no point in
adding the #ifdef's to this file ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com