> > > > +/** > > > + * fwctl_alloc_device - Allocate a fwctl > > > + * @parent: Physical device that provides the FW interface > > > + * @ops: Driver ops to register > > > + * @drv_struct: 'struct driver_fwctl' that holds the struct fwctl_device > > > + * @member: Name of the struct fwctl_device in @drv_struct > > > + * > > > + * This allocates and initializes the fwctl_device embedded in the drv_struct. > > > + * Upon success the pointer must be freed via fwctl_put(). Returns NULL on > > > + * failure. Returns a 'drv_struct *' on success, NULL on error. > > > + */ > > > +#define fwctl_alloc_device(parent, ops, drv_struct, member) \ > > > + container_of(_fwctl_alloc_device( \ > > > + parent, ops, \ > > > + sizeof(drv_struct) + \ > > > + BUILD_BUG_ON_ZERO( \ > > > + offsetof(drv_struct, member))), \ > > Doesn't that fire a build_bug when the member is at the start of drv_struct? > > Or do I have that backwards? > > BUILD_BUG_ON(true) == failure, evaluates to void > BUILD_BUG_ON_ZERO(true) == fails, evaluates to 0 > BUILD_BUG_ON_ZERO(false) == false, evaluates to 0 > > It is a bit confusing name, it is not ON_ZERO it is BUG_ON return ZERO Ah. That indeed got me. ouch. > > > Does container_of() safely handle a NULL? > > Generally no, nor does it handle ERR_PTR, but it does work for both if > the offset is 0. Ah. Good point, I'd neglected the zero offset meaning it is really just a fancy pointer type cast. > > The BUILD_BUG guarentees the 0 offset both so that the casting inside > _fwctl_alloc_device() works and we can use safely use container_of() > to enforce the type check. > > What do you think about writing it like this instead: > > #define fwctl_alloc_device(parent, ops, drv_struct, member) \ > ({ \ > static_assert(__same_type(struct fwctl_device, \ > ((drv_struct *)NULL)->member)); \ > static_assert(offsetof(drv_struct, member) == 0); \ > (drv_struct *)_fwctl_alloc_device(parent, ops, \ > sizeof(drv_struct)); \ > }) > > ? > > In some ways I like it better.. Seems more readable to me and avoids entertaining corners of the previous approach. Jonathan > > Thanks, > Jason