Re: [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols

[Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>]

On Wednesday 30 March 2016 16:49:03 Nick Clifton wrote:
> Hi Thomas.
> 
> > @@ -2632,8 +2632,9 @@ enum elf32_arm_stub_type
> > 
> >   {
> >   
> >     arm_stub_none,
> >     DEF_STUBS
> > 
> > +  max_stub_type,
> > 
> >     /* Note the first a8_veneer type.  */
> > 
> > -  arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond
> > +  arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond,
> > 
> >   };
> 
> Why do you need the extra comma at the end of the enum ?

There's no need, it's just an artifact from experiments I tried.

> 
> I don't like that "max_stub_type" is not actually the maximum possible
> value for an elf32_arn_stub_type enum.  The name is misleading.  Maybe
> you could use something like "last_non_veneer_stub".

Last is confusing because it's last + 1. To be fair, I don't think this should 
be a stub type, rather a const global variable. I can include that in the 
Cortex-A8 refactoring patch, what do you think?

> 
> > +  switch (stub_type)
> > +    {
> > +    default:
> > +      return FALSE;
> > +    }
> > +
> > +  abort ();  /* Should be unreachable.  */
> > +}
> 
> I presume that your intention is to extend the switch table later on,
> but there is no need for that now.  Just use:
> 
>    return FALSE;

Ok, will respin all patches following that same approach.

Best regards,

Thomas