This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] h8300 target breakpoint doesn't work on Simulator
At Fri, 11 Mar 2011 10:39:04 +0400,
Joel Brobecker wrote:
>
> > +2011-03-11 Yoshinori Sato <ysato@users.sourceforge.jp>
> > +
> > + * h8300-tdep.c (h8300_breakpoint_from_pc): Update to breakpoint
> > + instruction
> > +
>
> A few minor comments:
Thanks.
> > - /*static unsigned char breakpoint[] = { 0x7A, 0xFF }; *//* ??? */
> > - static unsigned char breakpoint[] = { 0x01, 0x80 }; /* Sleep */
> > + static const unsigned char sim_breakpoint[] = { 0x7A, 0xFF }; /* bpt */
> > + static const unsigned char breakpoint[] = { 0x57, 0x30 }; /* trap #3 */
>
> Can you remove a few spaces before the "/* bpt */" comment? It seems
> to me that the spaces are not really necessary, and removing them
> would make it a little closer to our soft-limit (70 characters).
>
> > - *lenptr = sizeof (breakpoint);
> > - return breakpoint;
> > + if (strcmp(target_shortname, "sim") == 0) {
> > + *lenptr = sizeof (sim_breakpoint);
> > + return sim_breakpoint;
> > + } else {
> > + *lenptr = sizeof (breakpoint);
> > + return breakpoint;
> > + }
> > }
>
> The proper style for braces in GDB is to put the curly brace on
> the next line. Also, we require a space before opening parens
> in function calls. Thus
>
> if (strcmp (target_shortname, "sim") == 0)
> {
> *lenptr = sizeof (sim_breakpoint);
> return sim_breakpoint;
> }
> else
> {
> *lenptr = sizeof (breakpoint);
> return breakpoint;
> }
Sorry. I mistake editor setting.
Cleanup.
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.12820
diff -u -r1.12820 ChangeLog
--- ChangeLog 16 Mar 2011 09:49:41 -0000 1.12820
+++ ChangeLog 16 Mar 2011 13:47:25 -0000
@@ -1,3 +1,8 @@
+2011-03-16 Yoshinori Sato <ysato@users.sourceforge.jp>
+
+ * h8300-tdep.c (h8300_breakpoint_from_pc): Update to breakpoint
+ instruction
+
2011-03-16 Phil Muldoon <pmuldoon@redhat.com>
* NEWS: Add Parameter sub-classing description.
Index: h8300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
retrieving revision 1.128
diff -u -r1.128 h8300-tdep.c
--- h8300-tdep.c 25 Jan 2011 12:13:20 -0000 1.128
+++ h8300-tdep.c 16 Mar 2011 13:47:25 -0000
@@ -1197,11 +1197,19 @@
h8300_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
int *lenptr)
{
- /*static unsigned char breakpoint[] = { 0x7A, 0xFF }; *//* ??? */
- static unsigned char breakpoint[] = { 0x01, 0x80 }; /* Sleep */
+ static const unsigned char sim_breakpoint[] = { 0x7A, 0xFF }; /*bpt*/
+ static const unsigned char breakpoint[] = { 0x57, 0x30 }; /*trap #3*/
- *lenptr = sizeof (breakpoint);
- return breakpoint;
+ if (strcmp(target_shortname, "sim") == 0)
+ {
+ *lenptr = sizeof (sim_breakpoint);
+ return sim_breakpoint;
+ }
+ else
+ {
+ *lenptr = sizeof (breakpoint);
+ return breakpoint;
+ }
}
static void
> I wonder if there isn't a better way to detect the sim, other
> than checking the target name. I don't know of any, but perhaps
> other maintainers might...
Hmm...
I think so.
However, I think it is rare case. I do not think that there is so an
enhanced necessity.
> --
> Joel
--
Yoshinori Sato
<ysato@users.sourceforge.jp>
===File ~/h8300-tdep.diff===================================
============================================================