Re: [PATCH 06/13] Add enum for mips breakpoint kinds

["Maciej W. Rozycki" <macro@imgtec.com>]

Pedro, Yao --

 I missed the MIPS parts previously, sorry.

On Thu, 27 Oct 2016, Pedro Alves wrote:

> > +/* Enum describing the different kinds of breakpoints.  */
> > +
> > +enum mips_breakpoint_kinds
> 
> IMO that should be singular.  Imagine you put one of these
> in a variable.  Like:
> 
>   enum mips_breakpoint_kinds kind;
> 
> This would be more natural, IMO:
> 
>   enum mips_breakpoint_kind kind;

 Agreed.

> > +{
> > +  /* 16-bit MIPS16 mode breakpoint */
> > +  MIPS_BP_KIND_16BIT_MIPS16 = 2,
> > +  /* 16-bit microMIPS mode breakpoint */
> > +  MIPS_BP_KIND_16BIT_MICROMIPS = 3,
> > +  /* 32-bit standard MIPS mode breakpoint */
> > +  MIPS_BP_KIND_32BIT = 4,
> > +  /* 32-bit microMIPS mode breakpoint */
> > +  MIPS_BP_KIND_32BIT_MICROMIPS = 5,
> 
> IMO a line break between these makes it much more readable.
> 
>   /* 16-bit MIPS16 mode breakpoint */
>   MIPS_BP_KIND_16BIT_MIPS16 = 2,
> 
>   /* 16-bit microMIPS mode breakpoint */
>   MIPS_BP_KIND_16BIT_MICROMIPS = 3,
> 
> etc.

 Indeed.  Also a full stop and two spaces at the end are due.

 Also how about calling them:

MIPS_BP_KIND_MIPS16
MIPS_BP_KIND_MICROMIPS16
MIPS_BP_KIND_MIPS32
MIPS_BP_KIND_MICROMIPS32

to make the names a bit shorter though still retaining the meaning?

  Maciej