From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17207 invoked by alias); 28 Oct 2004 04:55:15 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 17107 invoked from network); 28 Oct 2004 04:55:12 -0000 Received: from unknown (HELO legolas.inter.net.il) (192.114.186.24) by sourceware.org with SMTP; 28 Oct 2004 04:55:12 -0000 Received: from zaretski ([80.230.156.11]) by legolas.inter.net.il (MOS 3.5.3-GR) with ESMTP id CYU50538 (AUTH halo1); Thu, 28 Oct 2004 06:54:26 +0200 (IST) Date: Thu, 28 Oct 2004 04:55:00 -0000 From: "Eli Zaretskii" To: Jeff Johnston Message-ID: <01c4bca9$Blat.v2.2.2$adcffb00@zahav.net.il> Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=ISO-8859-1 CC: drow@false.org, cagney@gnu.org, gdb-patches@sources.redhat.com In-reply-to: <418022DE.204@redhat.com> (message from Jeff Johnston on Wed, 27 Oct 2004 18:36:14 -0400) Subject: Re: [RFA]: Watchpoints per thread patch Reply-to: Eli Zaretskii References: <4175A9C9.8040300@redhat.com> <41769FF3.7010801@gnu.org> <20041020173035.GA26622@nevyn.them.org> <418022DE.204@redhat.com> X-SW-Source: 2004-10/txt/msg00466.txt.bz2 > Date: Wed, 27 Oct 2004 18:36:14 -0400 > From: Jeff Johnston > Cc: Andrew Cagney , 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 *); > > + > +/* 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}.