On Tue, 21 Mar 2023, Alex Elder wrote: > On 3/21/23 1:34 PM, Menna Mahmoud wrote: > > Convert `to_gbphy_dev` and `to_gbphy_driver` macros into a > > static inline function. > > > > It is not great to have macros that use the `container_of` macro, > > because from looking at the definition one cannot tell what type > > it applies to. > > > > One can get the same benefit from an efficiency point of view > > by making an inline function. > > > > Suggested-by: Julia Lawall <julia.lawall@xxxxxxxx> > > Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@xxxxxxxxx> > > I'm sorry if this conflicts with what others have said. > > But the use of a macro (with a container_of() right-hand > side) to get at the structure containing a field pointer > is a widely-used idiom throughout the kernel. > > What you propose achieves the same result but I would > lean toward keeping it as a macro, mainly because it > is so common. Common is not necessarily good. Macros are less safe and less informative. julia > > -Alex > > --- > > changes in v2: > > -send patch as a single patch. > > -edit the name of struct object. > > -edit commit message. > > --- > > drivers/staging/greybus/gbphy.h | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/greybus/gbphy.h > > b/drivers/staging/greybus/gbphy.h > > index d4a225b76338..e7ba232bada1 100644 > > --- a/drivers/staging/greybus/gbphy.h > > +++ b/drivers/staging/greybus/gbphy.h > > @@ -15,7 +15,10 @@ struct gbphy_device { > > struct list_head list; > > struct device dev; > > }; > > -#define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev) > > +static inline struct gbphy_device *to_gbphy_dev(const struct device *_dev) > > +{ > > + return container_of(_dev, struct gbphy_device, dev); > > +} > > static inline void *gb_gbphy_get_data(struct gbphy_device *gdev) > > { > > @@ -43,7 +46,10 @@ struct gbphy_driver { > > struct device_driver driver; > > }; > > -#define to_gbphy_driver(d) container_of(d, struct gbphy_driver, driver) > > +static inline struct gbphy_driver *to_gbphy_driver(struct device_driver > > *drv) > > +{ > > + return container_of(drv, struct gbphy_driver, driver); > > +} > > int gb_gbphy_register_driver(struct gbphy_driver *driver, > > struct module *owner, const char *mod_name); > >