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 <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Michal Nazarewicz <[email protected]>
Cc: Hidehiro Kawai <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Johannes Berg <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Alexander Potapenko <[email protected]>
---
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: <[email protected]> )=-
-=( Web: http://www.mev.co.uk/ )=-
