On 23/05/17 14:23, Ian Abbott wrote:
On 23/05/17 13:02, Ian Abbott wrote:
On 23/05/17 12:24, Peter Zijlstra wrote:
On Tue, May 23, 2017 at 11:32:02AM +0100, Ian Abbott wrote:

#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).

We have BUILD_BUG_ON() that should be similar in functionality.

Yes, I think BUILD_BUG_ON_ZERO() will be better in this case to avoid
some baggage.  Also, __same_type() can be used for type checking.


BUILD_BUG_ON_ZERO() in the left-hand side of a comma expression produced
lots of warnings about it having no effect, so I tried using
BUILD_BUG_ON() instead as follows:

#define container_of(ptr, type, member) ({            \
    BUILD_BUG_ON(!__same_type((ptr), &((type *)0)->member));\
    ((type *)((char *)(ptr) - offsetof(type, member)));})

Unfortunately, this does result in compiler failures in places where the
types do not in fact match, for example `slab_show()` in
"mm/slab_common.c" (`list_entry()` uses `container_of()`):

static int slab_show(struct seq_file *m, void *p)
{
    struct kmem_cache *s = list_entry(p, struct kmem_cache,
root_caches_node);


I then changed it to allow `ptr` to be a `void *` too:

#define container_of(ptr, type, member) ({            \
    BUILD_BUG_ON(!__same_type((ptr), &((type *)0)->member) &&\
             !__same_type((ptr), void *));        \
    ((type *)((char *)(ptr) - offsetof(type, member)));})

Now it fails on `external_name()` in "fs/dcache.c":

static inline struct external_name *external_name(struct dentry *dentry)
{
    return container_of(dentry->d_name.name, struct external_name,
name[0]);
}

Here, `dentry->d_name.name` is of type `const unsigned char *` and
`name[0]` in `struct external_name` is of type `unsigned char`.  The
error is:

error: call to ‘__compiletime_assert_258’ declared with attribute error:
BUILD_BUG_ON failed: !__same_type((dentry->d_name.name), &((struct
external_name *)0)->name[0]) && !__same_type((dentry->d_name.name), void *)
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                      ^

I'm not quite sure why that is failing yet.

It fails because `const unsigned char *` is not compatible with `unsigned char *` as fast as `__builtin_types_compatible_p()` is concerned (quite rightly so). However, a simple change fixes that:

#define container_of(ptr, type, member) ({                              \
        BUILD_BUG_ON(!__same_type(*(ptr), ((type *)0)->member) &&    \
                     !__same_type((ptr), void *));                      \
        ((type *)((char *)(ptr) - offsetof(type, member)));})

That one seems to work fine (after including <linux/bug.h> in kernel.h and fixing up include/asm-generic/bug.h a little bit, which will be in a separate patch.

Thanks for the help guys.  v3 will be a series of 2 patches.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <[email protected]> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

Reply via email to