Messages in this thread | | | From | Michal Nazarewicz <> | Subject | Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() | Date | Mon, 22 May 2017 19:58:36 +0200 |
| |
On Mon, May 22 2017, Ian Abbott wrote: > If the first parameter of container_of() is a pointer to a > non-const-qualified array type (and the third parameter names a > non-const-qualified array member), the local variable __mptr will be > defined with a const-qualified array type. In ISO C, these types are > incompatible. They work as expected in GNU C, but some versions will > issue warnings. For example, GCC 4.9 produces the warning > "initialization from incompatible pointer type". > > Here is an example of where the problem occurs: > > ------------------------------------------------------- > #include <linux/kernel.h> > #include <linux/module.h> > > MODULE_LICENSE("GPL"); > > struct st { > int a; > char b[16]; > }; > > static int __init example_init(void) { > struct st t = { .a = 101, .b = "hello" }; > char (*p)[16] = &t.b; > struct st *x = container_of(p, struct st, b); > printk(KERN_DEBUG "%p %p\n", (void *)&t, (void *)x); > return 0; > } > > static void __exit example_exit(void) { > } > > module_init(example_init); > module_exit(example_exit); > ------------------------------------------------------- > > Building the module with gcc-4.9 results in these warnings (where '{m}' > is the module source and '{k}' is the kernel source): > > ------------------------------------------------------- > In file included from {m}/example.c:1:0: > {m}/example.c: In function ‘example_init’: > {k}/include/linux/kernel.h:854:48: warning: initialization from > incompatible pointer type > const typeof( ((type *)0)->member ) *__mptr = (ptr); \ > ^ > {m}/example.c:14:17: note: in expansion of macro ‘container_of’ > struct st *x = container_of(p, struct st, b); > ^ > {k}/include/linux/kernel.h:854:48: warning: (near initialization for > ‘x’) > const typeof( ((type *)0)->member ) *__mptr = (ptr); \ > ^ > {m}/example.c:14:17: note: in expansion of macro ‘container_of’ > struct st *x = container_of(p, struct st, b); > ^ > ------------------------------------------------------- > > Fix it by avoiding defining the __mptr variable. This also avoids other > GCC extensions. > > Signed-off-by: Ian Abbott <abbotti@mev.co.uk> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Michal Nazarewicz <mina86@mina86.com> > Cc: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> > Cc: Borislav Petkov <bp@suse.de> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk> > Cc: Johannes Berg <johannes.berg@intel.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Alexander Potapenko <glider@google.com> > --- > v2: Rebased and altered description to provide an example of when the > compiler warnings occur. v1 (from 2016-10-10) also modified a > 'container_of_safe()' macro that never made it out of "linux-next". > --- > include/linux/kernel.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 13bc08aba704..169fe6f51b7b 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -850,9 +850,8 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } > * @member: the name of the member within the struct. > * > */ > -#define container_of(ptr, type, member) ({ \ > - const typeof( ((type *)0)->member ) *__mptr = (ptr); \ > - (type *)( (char *)__mptr - offsetof(type,member) );}) > +#define container_of(ptr, type, member) \ > + ((type *)((char *)(ptr) - offsetof(type, member)))
Now the type of ptr is not checked though. Using your example, I can now write:
struct st t = { .a = 101, .b = "hello" }; int *p = &t.a; struct st *x = container_of(p, struct st, b);
and it will compile with no warnings. Previously it would fail. The best I can think of would be (not tested):
#define container_of(ptr, type, member) ( \ _Static_assert(__builtin_types_compatible_p( \ typeof(ptr), typeof( ((type *)0)->member )*), "WUT"), \ ((type *)((char *)(ptr) - offsetof(type, member))); \ )
or maybe:
#define container_of(ptr, type, member) ( \ _Static_assert(__builtin_types_compatible_p( \ typeof(*ptr), typeof( ((type *)0)->member )), "WUT"), \ ((type *)((char *)(ptr) - offsetof(type, member))); \ )
> > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ > #ifdef CONFIG_FTRACE_MCOUNT_RECORD
-- Best regards ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ «If at first you don’t succeed, give up skydiving»
| |