Re: [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct

[Jeff Garzik]

Ingo Molnar wrote:
> > > > > That cpumask[] should probably be cpumask[0], to document the
> > > > > aliasing to ->span and ->cpus properly.
> > > > If the comment wasn't sufficient documentation, I don't think that  
> > > > would help :(
> > > It's a visual helper: it matches up with how we do these 'zero size  
> > > array means dynamic structure continuation' tricks generally.
> > >
> > > I first mis-parsed the code for a second when seeing cpumask[].  
> > > cpumask[0] stands out like a sore thumb. And we dont read comments  
> > > anyway ;-)
> > >
> > > Jeff, i suspect you found this because you are working on something  
> > > rather interesting? :) If yes, would it help your project if we did  
> > > the cpumask[0] cleanup and pushed it upstream immediately?
> > I think cpumask[0] would be more clear and consistent with the 
> > rest of the kernel.
> >
> > But unfortunately for the twin projects of (a) static analysis and 
> > checking with 'sparse', and (b) compiling under another compiler, 
> > VLA-in-middle-of-struct is a killer in either case.
>
> even if at the end of the struct?

Putting the VLA at the end of the struct would be a huge help, yes.

For example, struct sched_group and struct sched_domain are OK as-is 
(though "[0]" would be preferred).

It is the definition of struct static_sched_group and struct 
static_sched_domain that creates the problem, because with the bitmap 
following cpumask[] and span[], the VLA is no longer at the end of the 
struct.

VLA-in-the-middle raises the complexity required of the compiler quite a 
bit.  As a result, VLA-in-middle is not implemented in sparse or clang 
(LLVM's C front-end and static analyzer).

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/