Re: [PATCH] cgroup: reorder flexible array members of struct cgroup_root

From: Tejun Heo
Date: Sat Oct 21 2017 - 11:33:04 EST


Hello, Nick.

On Fri, Oct 20, 2017 at 12:15:55AM -0700, Nick Desaulniers wrote:
> > This is silly tho. We know the the root group embedded there won't
> > have any ancestor_ids.
>
> Sure, but struct cgroup_root is still declared as having a struct
> cgroup not declared as the final member.

Why is that a problem tho? We know that it doesn't have any flexible
array member so there's no storage allocated to it.

> > Also, in general, nothing prevents us from
> > doing something like the following.
> >
> > struct outer_struct {
> > blah blah;
> > struct inner_struct_with_flexible_array_member inner;
> > unsigned long storage_for_flexible_array[NR_ENTRIES];
> > blah blah;
> > };
>
> At that point, then why have the struct with flexible array member in
> the first place?

Because there are different ways to use the struct?

> >> or specific location of the member cgrp within struct cgroup_root AFAICT.
> > I think we should just silence the bogus warning.
>
> Is the order of the members actually important? Otherwise it seems
> that we're taking advantage of a GNU C extension for no real reason,
> which is what I'm trying to avoid. Please reconsider.

Here, not necessarily but I don't want to move it for a bogus reason.
Why would we disallow embedding structs with flexible members in the
middle when it can be done and is useful? If we want to discuss
whether we want to avoid such usages in the kernel (but why?), sure,
let's have that discussion but we can't decide that on "clang warns on
it by default".

Thanks.

--
tejun