From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3399 invoked by alias); 20 Jun 2009 22:01:15 -0000 Received: (qmail 2111 invoked by uid 22791); 20 Jun 2009 22:01:09 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 20 Jun 2009 22:01:01 +0000 Received: (qmail 27352 invoked from network); 20 Jun 2009 22:00:59 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 20 Jun 2009 22:00:59 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [patch] gdbserver: Add support for Z0/Z1 packets Date: Sat, 20 Jun 2009 22:01:00 -0000 User-Agent: KMail/1.9.10 Cc: Aleksandar Ristovski , Doug Evans References: <4A3B98EC.70209@qnx.com> In-Reply-To: <4A3B98EC.70209@qnx.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200906202301.52782.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-06/txt/msg00539.txt.bz2 On Friday 19 June 2009 14:55:56, Aleksandar Ristovski wrote: > RCS file: /cvs/src/src/gdb/gdbserver/target.h,v > retrieving revision 1.36 > diff -u -p -r1.36 target.h > --- target.h=A0=A0=A0=A01 Apr 2009 22:50:24 -0000=A0=A0=A0=A0=A0=A0=A01.36 > +++ target.h=A0=A0=A0=A019 Jun 2009 13:47:44 -0000 > @@ -213,6 +213,14 @@ struct target_ops > =A0 =A0int (*read_auxv) (CORE_ADDR offset, unsigned char *myaddr, > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0unsigned int len); > =A0 > + =A0/* Insert and remove breakpoint. =A0Argument TYPE can be: > + =A0 =A0 =A0 '0' =3D software-breakpoint > + =A0 =A0 =A0 '1' =3D hardware-breakpoint=20 ^ spurious space > + =A0 =A0 Returns 0 if successful, 1 otherwise. =A0*/ > + > + =A0int (*insert_breakpoint) (char type, CORE_ADDR addr); > + =A0int (*remove_breakpoint) (char type, CORE_ADDR addr); Z0 and Z1 breakpoints also take a 'len' argument, just like Z2-Z4. You should also pass those down. But, Let's take a step back --- why not just rename the insert_watchpoint|remove_watchpoint functions to insert_point,remove_point, and relax the type checks in server.c: if (the_target->insert_watchpoint =3D=3D NULL || (type < '2' || type > '4')) { ... to also allow type =3D=3D '0' or '1' ? Looking at your nto-low.c file in the original patch, you were doing this: +static int +nto_insert_breakpoint (char type, CORE_ADDR addr) ^^^^^^^^^^ +{ + TRACE ("%s\n", __func__); + + return nto_insert_watchpoint (type, addr, 1); ^^^^^^^^^^ +} In the latest patch, you're doing this: +static int +nto_insert_breakpoint (char type, CORE_ADDR addr) +{ + TRACE ("%s\n", __func__); + switch (type) + { + case '0': /* software-breakpoint */ + return nto_breakpoint (addr, _DEBUG_BREAK_EXEC, 0); + case '1': /* hardware-breakpoint */ + return nto_breakpoint (addr, _DEBUG_BREAK_EXEC | _DEBUG_BREAK_HW, 0); + } + return 1; /* Not supported. */ +} +static int +nto_insert_watchpoint (char type, CORE_ADDR addr, int len) +{ + int wtype =3D _DEBUG_BREAK_HW; /* Always request HW. */ + + TRACE ("%s type:%c\n", __func__, type); + switch (type) + { + case '2': /* write watchpoint */ + wtype |=3D _DEBUG_BREAK_RW; + break; + case '3': /* read watchpoint */ + wtype |=3D _DEBUG_BREAK_RD + break; + case '4': /* access watchpoint */ + wtype |=3D _DEBUG_BREAK_RW; + break; + default: + return 1; /* Not supported. */ + } + return nto_breakpoint (addr, wtype, 0); +} That is, still always deferring to a single function (nto_breakpoint). This suggests that the insert_breakpoint and insert_watchpoint interface distintion is unnecessary. --=20 Pedro Alves