This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Add support for Morpho ms1 processor
On Fri, 12 Aug 2005 12:35:54 +0200 (CEST)
Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > + if (regnum >= 0 &&
> > + regnum < sizeof (register_names) / sizeof (register_names[0]))
> > + {
> > + return register_names[regnum];
> > + }
> > + else
> > + internal_error (__FILE__, __LINE__,
> > + _("ms1_register_name: illegal register number %d"), regnum);
> > +}
>
> Please consider usin ARRAY_SIZE. I'd also use gdb_assert() here
> instead of the excplicit internal_error(), but that's a bit of a
> personal preference I assume. Please use invalid instead of illegal.
I'm okay with gdb_assert(). That allows us to condense the above bit of
code to just:
gdb_assert (regnum >= 0 && regnum < ARRAY_SIZE (register_names));
return register_names[regnum];
The other benefit to using gdb_assert() is that there's one less string
for the i18n translators to worry about.
> > +static const unsigned char *
> > +ms1_breakpoint_from_pc (CORE_ADDR *bp_addr, int *bp_size)
> > +{
> > + static char breakpoint[] = { 0x68, 0, 0, 0 };
> > +
> > + *bp_size = 4;
> > + return breakpoint;
> > +}
>
> Please use gdb_byte here too.
Done. (I've changed the return type of ms1_breakpoint_from_pc() and
the declaration of breakpoint[] in my sources.)
> Otherwise this looks great!
Thank you for reviewing this patch.
Kevin