Re: [RFA]: Watchpoints per thread patch

["Eli Zaretskii" <eliz@gnu.org>]

> Date: Wed, 27 Oct 2004 18:36:14 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> Cc: Andrew Cagney <cagney@gnu.org>, gdb-patches@sources.redhat.com
> 
> The attached patch is the rework of my original attempt.  It no longer uses 
> configuration or magic defines.  Per Mark's suggestion, it uses an observer to 
> handle inserting watchpoints on a new thread and only the low-level code knows 
> about inserting/removing watchpoints on all threads.
> 
> Ok to commit?

A few comments:

> +/* External function to insert all existing watchpoints on a newly
> +   attached thread.  IWPFN is a callback function to perform
> +   the target insert watchpoint.  This function is used to support
> +   platforms whereby a watchpoint must be inserted/removed on each
> +   individual thread (e.g. ia64-linux and s390-linux).  For
> +   ia64 and s390 linux, this function is called via a new thread
> +   observer.  */

In this comment, the word "whereby" should be replaced by "where", I
think.

> --- target.h	8 Oct 2004 20:29:55 -0000	1.65
> +++ target.h	27 Oct 2004 21:43:51 -0000
> @@ -178,6 +178,15 @@ extern char *target_signal_to_name (enum
>  /* Given a name (SIGHUP, etc.), return its signal.  */
>  enum target_signal target_signal_from_name (char *);
>  \f
> +
> +/* Watchpoint specification.  */
> +struct target_watchpoint
> +  {
> +    CORE_ADDR addr;
> +    int len;
> +    int type;
> +  };
> +

Why do we put on target.h, which is a general header, a definition of
a struct used only on certain platforms?

> --- doc/observer.texi	1 Sep 2004 17:59:37 -0000	1.8
> +++ doc/observer.texi	27 Oct 2004 21:43:51 -0000
> @@ -95,3 +95,7 @@ inferior, and before any information on 
>  The specified shared library has been discovered to be unloaded.
>  @end deftypefun
>  
> +@deftypefun void new_thread (ptid_t @var{ptid})
> +A new thread has been attached to.

"A new thread has been attached" to what?  The description of the
observer should at least reference the argument @var{ptid}.