Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1647926
| From | Ian Abbott <abbotti@mev.co.uk> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() |
| Date | 2017-05-23 12:40 +0200 |
| Message-ID | <tKfol-1W6-9@gated-at.bofh.it> (permalink) |
| References | <tJXhM-7lx-7@gated-at.bofh.it> <tJZMC-gQ-13@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 22/05/17 18:58, Michal Nazarewicz wrote:
> 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))); \
> )
It's a fine suggestion (if more parentheses are added), but
_Static_assert is a C11 feature, and I thought the kernel was using
gnu89 (unless it's been updated since).
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott <abbotti@mev.co.uk> - 2017-05-22 17:20 +0200
Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() Michal Nazarewicz <mina86@mina86.com> - 2017-05-22 20:00 +0200
Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott <abbotti@mev.co.uk> - 2017-05-23 12:40 +0200
Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() Peter Zijlstra <peterz@infradead.org> - 2017-05-23 13:30 +0200
Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott <abbotti@mev.co.uk> - 2017-05-23 14:10 +0200
Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott <abbotti@mev.co.uk> - 2017-05-23 15:40 +0200
Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott <abbotti@mev.co.uk> - 2017-05-23 16:20 +0200
csiph-web