From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23266 invoked by alias); 23 Jun 2009 15:18:52 -0000 Received: (qmail 23255 invoked by uid 22791); 23 Jun 2009 15:18:52 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from qnxmail.qnx.com (HELO qnxmail.qnx.com) (209.226.137.76) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Jun 2009 15:18:45 +0000 Received: from Nebula.ott.qnx.com (nebula.ott.qnx.com [10.42.3.30]) by hub.ott.qnx.com (8.9.3/8.9.3) with ESMTP id LAA15008; Tue, 23 Jun 2009 11:18:37 -0400 Received: from [127.0.0.1] ([10.42.100.129]) by Nebula.ott.qnx.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 23 Jun 2009 11:18:40 -0400 Message-ID: <4A40F226.4080909@qnx.com> Date: Tue, 23 Jun 2009 15:18:00 -0000 From: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.22 (Windows/20090605) MIME-Version: 1.0 To: gdb-patches@sourceware.org CC: Pedro Alves , Doug Evans Subject: Re: [patch] gdbserver: Add support for Z0/Z1 packets References: <200906202301.52782.pedro@codesourcery.com> <4A3FDDCA.2010106@qnx.com> <200906222346.54263.pedro@codesourcery.com> In-Reply-To: <200906222346.54263.pedro@codesourcery.com> Content-Type: multipart/mixed; boundary="------------070600000307040206020805" 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/msg00607.txt.bz2 This is a multi-part message in MIME format. --------------070600000307040206020805 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1312 Pedro Alves wrote: > On Monday 22 June 2009 20:38:50, Aleksandar Ristovski wrote: > >>> 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: >> That was my initial implementation, prior to proposing the >> change. Then I looked at target ops in gdb; there we have >> two different functions for breakpoint and watchpoint so I >> followed that logic (even though the logic there seems to be >> incomplete: there is a pair for hw and non-hw breakponts but >> only one pair for watchpoints). > > That's because software watchpoints aren't "inserted". Yes, silly me. > >> But either way is fine with me - just let me know. > > I'd prefer the approach I suggested, and worry about splitting > the breakpoints from watchpoints API if/when we actually need it. > Ok, then that version is committed. I attached what I committed. ChangeLog: * server.c (process_serial_event): Add support for Z0 and Z1 packet. * target.h: Comment for *_watchpoint to make it clear the functions can get types '0' and '1'. Thanks, -- Aleksandar Ristovski QNX Software Systems --------------070600000307040206020805 Content-Type: text/x-patch; name="gdbserver-Z0Z1support-20090623.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="gdbserver-Z0Z1support-20090623.diff" Content-length: 3699 Index: server.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/server.c,v retrieving revision 1.98 diff -u -p -r1.98 server.c --- server.c 19 Jun 2009 13:35:35 -0000 1.98 +++ server.c 23 Jun 2009 15:06:21 -0000 @@ -2371,66 +2371,56 @@ process_serial_event (void) signal = 0; myresume (own_buf, 1, signal); break; - case 'Z': + case 'Z': /* insert_ ... */ + /* Fallthrough. */ + case 'z': /* remove_ ... */ { char *lenptr; char *dataptr; CORE_ADDR addr = strtoul (&own_buf[3], &lenptr, 16); int len = strtol (lenptr + 1, &dataptr, 16); char type = own_buf[1]; + int res; + const int insert_ = ch == 'Z'; + + /* Type: '0' - software-breakpoint + '1' - hardware-breakpoint + '2' - write watchpoint + '3' - read watchpoint + '4' - access watchpoint */ if (the_target->insert_watchpoint == NULL - || (type < '2' || type > '4')) - { - /* No watchpoint support or not a watchpoint command; - unrecognized either way. */ - own_buf[0] = '\0'; - } + || the_target->remove_watchpoint == NULL) + res = 1; /* Not supported. */ else - { - int res; - - require_running (own_buf); - res = (*the_target->insert_watchpoint) (type, addr, len); - if (res == 0) - write_ok (own_buf); - else if (res == 1) - /* Unsupported. */ - own_buf[0] = '\0'; - else - write_enn (own_buf); - } - break; - } - case 'z': - { - char *lenptr; - char *dataptr; - CORE_ADDR addr = strtoul (&own_buf[3], &lenptr, 16); - int len = strtol (lenptr + 1, &dataptr, 16); - char type = own_buf[1]; + switch (type) + { + case '2': + /* Fallthrough. */ + case '3': + /* Fallthrough. */ + case '4': + require_running (own_buf); + /* Fallthrough. */ + case '0': + /* Fallthrough. */ + case '1': + res = insert_ ? (*the_target->insert_watchpoint) (type, addr, + len) + : (*the_target->remove_watchpoint) (type, addr, + len); + break; + default: + res = -1; /* Unrecognized type. */ + } - if (the_target->remove_watchpoint == NULL - || (type < '2' || type > '4')) - { - /* No watchpoint support or not a watchpoint command; - unrecognized either way. */ - own_buf[0] = '\0'; - } + if (res == 0) + write_ok (own_buf); + else if (res == 1) + /* Unsupported. */ + own_buf[0] = '\0'; else - { - int res; - - require_running (own_buf); - res = (*the_target->remove_watchpoint) (type, addr, len); - if (res == 0) - write_ok (own_buf); - else if (res == 1) - /* Unsupported. */ - own_buf[0] = '\0'; - else - write_enn (own_buf); - } + write_enn (own_buf); break; } case 'k': Index: target.h =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/target.h,v retrieving revision 1.37 diff -u -p -r1.37 target.h --- target.h 19 Jun 2009 13:35:35 -0000 1.37 +++ target.h 23 Jun 2009 15:06:21 -0000 @@ -216,10 +216,11 @@ struct target_ops /* Insert and remove a hardware watchpoint. Returns 0 on success, -1 on failure and 1 on unsupported. The type is coded as follows: - 2 = write watchpoint - 3 = read watchpoint - 4 = access watchpoint - */ + '0' - software-breakpoint + '1' - hardware-breakpoint + '2' - write watchpoint + '3' - read watchpoint + '4' - access watchpoint */ int (*insert_watchpoint) (char type, CORE_ADDR addr, int len); int (*remove_watchpoint) (char type, CORE_ADDR addr, int len); --------------070600000307040206020805--