This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] gdbserver: Add support for Z0/Z1 packets
Pedro Alves wrote:
On Tuesday 23 June 2009 16:17:58, Aleksandar Ristovski wrote:
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:
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.
Well, we had never seen "that" version
Ok, to rectify this I am attaching two versions: one if I
revert the changes I committed and the other is diff to what
is in now.
> ... and you bypassed the "rename" suggestion...
I did not do any renaming - I think it is not terribly
confusing since both in target.h comment and server.c 'Z'
case it is made very clear that it handles both breakpoints
and watchpoints (i.e. I don't find it any clearer if it was
called "insert_point"... it would still require reading the
comment in target.h)
Would you care to explain why are watchpoints guarded on
require_running and breakpoints aren't? It's not super
obvious to me.
Both proposed versions now check for require_running for any
kind of breakpoint.
ChangeLog - diff to what is in now.
* server.c (process_serial_event): Treat all types of
break/watch-points the same.
ChangeLog - diff to what was before my commit:
* 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'.
(attached first diff to what is in already and then diff to
what was before commit).
Thanks,
--
Aleksandar Ristovski
QNX Software Systems
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.99
diff -u -p -r1.99 server.c
--- server.c 23 Jun 2009 15:12:44 -0000 1.99
+++ server.c 24 Jun 2009 15:36:11 -0000
@@ -2381,7 +2381,6 @@ process_serial_event (void)
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
@@ -2392,27 +2391,16 @@ process_serial_event (void)
if (the_target->insert_watchpoint == NULL
|| the_target->remove_watchpoint == NULL)
res = 1; /* Not supported. */
+ else if (type >= '0' && type <= '4')
+ {
+ const int insert_ = ch == 'Z';
+
+ require_running (own_buf);
+ res = insert_ ? (*the_target->insert_watchpoint) (type, addr, len)
+ : (*the_target->remove_watchpoint) (type, addr, len);
+ }
else
- 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. */
- }
+ res = -1; /* Unrecognized type. */
if (res == 0)
write_ok (own_buf);
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 24 Jun 2009 17:13:04 -0000
@@ -2371,66 +2371,44 @@ 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;
+
+ /* 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';
- }
- else
+ || the_target->remove_watchpoint == NULL)
+ res = 1; /* Not supported. */
+ else if (type >= '0' && type <= '4')
{
- 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';
+ if (ch == 'Z')
+ res = (*the_target->insert_watchpoint) (type, addr, len);
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];
-
- 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';
+ res = (*the_target->remove_watchpoint) (type, addr, len);
}
else
- {
- int res;
+ res = -1; /* Unrecognized type. */
- 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);
- }
+ if (res == 0)
+ write_ok (own_buf);
+ else if (res == 1)
+ /* Unsupported. */
+ own_buf[0] = '\0';
+ else
+ write_enn (own_buf);
break;
}
case 'k':
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
retrieving revision 1.37
retrieving revision 1.38
diff -u -p -r1.37 -r1.38
--- target.h 19 Jun 2009 13:35:35 -0000 1.37
+++ target.h 23 Jun 2009 15:12:44 -0000 1.38
@@ -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);