| Message ID | CANeU7Q=p=kcHCZ2uupc+7e_Gg5uYpe2b2aoS+8nAwyK+jxLtqw@mail.gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On Tue, 2016-11-15 at 09:00 +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, That looks fine to me. If you want to merge that one, then: Reviewed-by: Jeff Layton <jlayton@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.) -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 15, 2016 at 10:07 AM, Jeff Layton <jlayton@redhat.com> wrote: > > That looks fine to me. If you want to merge that one, then: > > Reviewed-by: Jeff Layton <jlayton@redhat.com> The patches is applied on "chrisl" repo and pushed out. Actually I want to merge the patch having you as the author. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2016-11-17 at 06:28 +0800, Christopher Li wrote: > On Tue, Nov 15, 2016 at 10:07 AM, Jeff Layton <jlayton@redhat.com> wrote: > > > > > > That looks fine to me. If you want to merge that one, then: > > > > Reviewed-by: Jeff Layton <jlayton@redhat.com> > > The patches is applied on "chrisl" repo and pushed out. > > Actually I want to merge the patch having you as the author. > > Chris Looks good to me. Thanks!
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,