On Mon, Jan 28, 2019 at 06:51:09PM +0200, Leon Romanovsky wrote:
> On Mon, Jan 28, 2019 at 08:27:17AM -0800, Bart Van Assche wrote:
> > On Mon, 2019-01-28 at 17:20 +0200, Leon Romanovsky wrote:
> > > On Mon, Jan 28, 2019 at 07:01:41AM -0800, Bart Van Assche wrote:
> > > > On 1/28/19 6:14 AM, Leon Romanovsky wrote:
> > > > > -struct ib_device *ib_alloc_device(size_t size);
> > > > > +void *_ib_alloc_device(size_t size);
> > > > > +#define ib_alloc_device(drv_struct, member) \
> > > > > + (BUILD_BUG_ON_ZERO(offsetof(struct drv_struct, member)) + \
> > > > > + BUILD_BUG_ON_ZERO(!__builtin_types_compatible_p( \
> > > > > + typeof(((struct drv_struct *)NULL)->member), \
> > > > > + struct ib_device)) + \
> > > > > + _ib_alloc_device(sizeof(struct drv_struct)))
> > > > > +
> > > >
> > > > Hi Leon,
> > > >
> > > > Have you considered to use a comma expression and BUILD_BUG_ON() instead
> > > > of adding the BUILD_BUG_ON_ZERO() results to the _ib_alloc_device()
> > > > return value? I think that would result in slightly easier to read code
> > > > and also in code that reflects the intent better.
> > >
> > > I wanted to make sure that macro is used as assignment and returns
> > > value, in such case it should be art of expression, hence in success
> > > it is evaluated to 0.
> > >
> > > I will very happy to receive code snippet for the same with BUILD_BUG_ON().
> >
> > Hi Leon,
> >
> > How about the following macro definition, which uses BUILD_BUG_ON() instead of
> > BUILD_BUG_ON_ZERO() and which uses __same_type() instead of
> > __builtin_types_compatible_p() (compile-tested only):
>
> Strange, i tried __same_type() too and the second typeof didn't work for
> me. But in your macro it works, maybe I missed some brackets.
That type checking should use container_of, none of the versions here
are entirely safe.
Personally, I think the _ZERO is a a lot more understandable than
having to add all the ({;}) line noise to make it work as a comma
expression, this is why _ZERO exists.
struct ib_device *_ib_alloc_device(size_t total_size);
#define ib_alloc_device(drv_struct, member) \
container_of(_ib_alloc_device(sizeof(struct drv_struct) + \
BUILD_BUG_ON_ZERO(offsetof( \
struct drv_struct, member))), \
drv_struct, member)
container_of does the typechecking and forces the result of the
expression to be of type drv_struct, not void.
Jason
