Re: [PATCH] Handle SForced in storage_modifiers

[Josh Triplett <josh@joshtriplett.org>]

On Tue, Nov 15, 2016 at 09:00:30AM +0800, Christopher Li wrote:
> On Tue, Nov 15, 2016 at 4:17 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > The problem is the _size_ of the array. Without that initializer for
> > SForced case, it is one entry too small, and you get a random access
> > past the end of the array.
> >
> > The patch is definitely correct.
> >
> 
> Yes, the patch is definitely correct. It is a good catch.
> 
> I purpose a slightly different way to fix it. I think it is better just give the
> array of a size instead of using the designated initializer to determine the
> array size. Sequential initializer to determine the array size is fine.
> 
> Jeff, how about some thing like this:
> 
> Chris
> 
> diff --git a/parse.c b/parse.c
> index 66f9353..a01ba00 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -109,7 +109,7 @@ enum {
>  };
> 
>  enum {
> -       SNone = 0, STypedef, SAuto, SRegister, SExtern, SStatic, SForced
> +       SNone = 0, STypedef, SAuto, SRegister, SExtern, SStatic, SForced, SMax
>  };
> 
>  static struct symbol_op typedef_op = {
> @@ -1279,7 +1279,7 @@ static const char *storage_class[] =
> 
>  static unsigned long storage_modifiers(struct decl_state *ctx)
>  {
> -       static unsigned long mod[] =
> +       static unsigned long mod[SMax] =
>         {
>                 [SAuto] = MOD_AUTO,
>                 [SExtern] = MOD_EXTERN,

I like this much better as well.

(Having "maximum" entries in enums does sadly break warning options like
-Wswitch-enum, but it makes cases like this much better.)