Re: [RFC] Add support for Morpho ms1 processor

[Kevin Buettner <kevinb@redhat.com>]

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