Date: Wed, 16 Sep 2009 22:41:57 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Ferenc Wagner <wferi@niif.hu>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Make safe some helpers building on container_of
Message-Id: <20090916224157.cfa621c4.akpm@linux-foundation.org>
In-Reply-To: <20090914082700.8550.94087.stgit@szonett.ki.iif.hu>
References: <20090914082700.8550.94087.stgit@szonett.ki.iif.hu>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1824
Lines: 44

On Mon, 14 Sep 2009 10:27:00 +0200 Ferenc Wagner <wferi@niif.hu> wrote:

> For example, drivers/base/core.c contains
> 
> #define to_root_device(dev) container_of(dev, struct root_device, dev)
> 
> which works fine as long as the 'to_root_device' macro is always
> applied to a variable called 'dev', as it is the case in the current
> kernel sources.  However, it breaks as soon as it gets applied to a
> variable of any other name, as the name of the variable is also
> substituted into the third argument of the 'container_of' macro, which
> really should stay 'dev' in the above case.
> 
> This patch renames the real macro arguments in all 5 such constructs
> found by git-grep -E '#define.*container_of *\( *([^ ,]+) *,.*, *\1 *\)',
> which may have missed some similar dangerous constructs, of course.
> So these changes probably cross all possible boundaries of responsibility,
> and I do not know how to best handle it, suggestions welcome.
> 
> Btw. this dangerous idom is also popularised by the excellent Linux Device
> Drivers book (3rd edition, chapter 14, bottom of page 383).
> 
> ...
>
> -#define EVT_TO_HPET_DEV(evt) container_of(evt, struct hpet_dev, evt)
> +#define EVT_TO_HPET_DEV(evtdev) container_of(evtdev, struct hpet_dev, evt)

There is no reason whatsoever for implementing these things as macros
and for the life of me I don't understand why people do this.

If we're going to fix these things then how about we turn them into
C functions thereby making them even safer?


y:/usr/src/linux-2.6.31> grep -r 'define.*container_of' . | wc -l
347

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