All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Jeff Garzik <jeff@garzik.org>, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Mike Travis <travis@sgi.com>, LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	roland@redhat.com
Subject: Re: [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct
Date: Tue, 12 May 2009 15:03:44 +0100	[thread overview]
Message-ID: <20090512140344.GO8633@ZenIV.linux.org.uk> (raw)
In-Reply-To: <200905122304.52395.rusty@rustcorp.com.au>

On Tue, May 12, 2009 at 11:04:51PM +0930, Rusty Russell wrote:
> On Mon, 11 May 2009 12:39:54 am Jeff Garzik wrote:
> > On Sun, May 10, 2009 at 06:19:40PM +0930, Rusty Russell wrote:
> > > Yeah, it's kinda nasty.  Generally, sched_group is dynamically allocated,
> > > so we just allocate sizeof(struct sched_group) + size of nr_cpu_ids bits.
> > >
> > > These ones are static, and it was easier to put this hack in than make
> > > them dynamic.  There's nothing wrong with it, until we really want
> > > NR_CPUS == bignum, or we want to get rid of NR_CPUS altogether for
> > > CONFIG_CPUMASKS_OFFSTACK (which would be very clean, but not clearly
> > > worthwhile).
> >
> > Nothing wrong with it, except
> >
> > - C99 only defines variable-length automatic arrays
> > - VLA in the middle of a struct are difficult to optimize
> > - gcc's VLA handling WILL change, as gcc docs state
> > - other compilers -- and sparse -- puke all over VLAs, making
> >   static analysis impossible for all code with this weirdism
> 
> Jeff, you seem confused.  In my copy of the standard, you'd know this is called 
> a "flexible array member"; it's not a variable length array.  The only GCC 
> specific issue I can find here is that you're not normally allowed to embed 
> structs with them in another struct (according to the gcc docs; I can't 
> actually find this clearly stated in the standard).

6.7.2.1p2.  It's a separate issue from revolting gcc extension that *do*
allow VLA-in-the-middle-of-struct.  And I mean real VLA, not flex array
member :-/

> Anyway, since [] is C99, I thought it preferable to [0] which is a gcc 
> extension.  However, if C99 is really so braindead as to disallow this fairly 
> standard trick, so I'm happy to go with the gcc extension.[1]

No.  There's a standard way to do that in C99; you can put a struct with
that thing into a union.  So correct way to force enought storage for
such an object is

union {
	struct has_flex_array_member foo;
	char [how_much_space_do_I_want];
} bar;

Unions with overlapping members are fine.  Structures are not.

  reply	other threads:[~2009-05-12 14:04 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-08 18:48 [PATCH 1/2] kernel/{sched,smp}.c: fix static decl prior to struct declaration Jeff Garzik
2009-05-08 18:50 ` [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct Jeff Garzik
2009-05-08 19:09   ` Ingo Molnar
2009-05-10  8:49     ` Rusty Russell
2009-05-10 15:09       ` Jeff Garzik
2009-05-12 13:34         ` Rusty Russell
2009-05-12 14:03           ` Al Viro [this message]
2009-05-13  2:12             ` Rusty Russell
2009-05-13  2:31               ` Jeff Garzik
2009-05-13  5:36               ` Al Viro
2009-05-13  6:49                 ` [PATCH] sched: avoid flexible array member inside struct (gcc extension) Rusty Russell
2009-05-13 13:51                   ` [tip:sched/urgent] " tip-bot for Rusty Russell
2009-05-11 10:58       ` [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct Ingo Molnar
2009-05-11 20:43         ` Jeff Garzik
2009-05-11 20:49           ` Ingo Molnar
2009-05-12  2:24             ` Jeff Garzik
2009-05-12  8:54               ` Ingo Molnar
2009-05-08 19:04 ` [PATCH 1/2] kernel/{sched,smp}.c: fix static decl prior to struct declaration Ingo Molnar
2009-05-08 19:08   ` Jeff Garzik
2009-05-08 19:13     ` Ingo Molnar
2009-05-08 19:38 ` [PATCH 1/2 v2] " Jeff Garzik
2009-05-11 11:26   ` Ingo Molnar
2009-05-11 20:06     ` Jeff Garzik
2009-05-11 20:51       ` Ingo Molnar
2009-05-11 11:30   ` [tip:sched/core] kernel/{sched, smp}.c: " tip-bot for Jeff Garzik
2009-05-11 20:02   ` [PATCH 1/2 v3] kernel/{sched,smp}.c: " Jeff Garzik
2009-05-11 21:51     ` [tip:sched/urgent] kernel/{sched, smp}.c: " tip-bot for Jeff Garzik
2009-05-11 21:56       ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090512140344.GO8633@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=travis@sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.