[PATCH] staging: rtl8723bs: use inline functions for dev_to_sdio_func

[PATCH] staging: rtl8723bs: use inline functions for dev_to_sdio_func

[Menna Mahmoud]

Convert `dev_to_sdio_func` macro into a static inline function.
it is not great to have macro that use `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@inria.fr>
Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 490431484524..7ee821dbbae0 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -10,7 +10,10 @@
 #include <linux/jiffies.h>
 
 #ifndef dev_to_sdio_func
-#define dev_to_sdio_func(d)     container_of(d, struct sdio_func, dev)
+static inline struct sdio_func *dev_to_sdio_func(struct device *d)
+{
+	return container_of(d, struct sdio_func, dev);
+}
 #endif
 
 static const struct sdio_device_id sdio_ids[] = {
-- 
2.34.1

Re: [PATCH] staging: rtl8723bs: use inline functions for dev_to_sdio_func

[Greg KH]

On Mon, Mar 20, 2023 at 12:34:41PM +0200, Menna Mahmoud wrote:
> Convert `dev_to_sdio_func` macro into a static inline function.
> it is not great to have macro that use `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@inria.fr>
> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> ---
>  drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> index 490431484524..7ee821dbbae0 100644
> --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> @@ -10,7 +10,10 @@
>  #include <linux/jiffies.h>
>  
>  #ifndef dev_to_sdio_func
> -#define dev_to_sdio_func(d)     container_of(d, struct sdio_func, dev)
> +static inline struct sdio_func *dev_to_sdio_func(struct device *d)
> +{
> +	return container_of(d, struct sdio_func, dev);
> +}
>  #endif

Why is the "#ifndef" check still needed now?  Really it was never
needed, but now would be a great time to remove it as it doubly does not
make any sense here.

Oh wait, no, this whole thing can just be removed entirely, right?
There already is a dev_to_sdio_func macro defined, so the #ifndef check
catches that so your change doesn't actually modify any code that is
used.  So this should all be removed, not changed to an inline function.

thanks,

greg k-h

Re: [PATCH] staging: rtl8723bs: use inline functions for dev_to_sdio_func

[Menna Mahmoud]

On ٢٠‏/٣‏/٢٠٢٣ ١٢:٤٣, Greg KH wrote:
> On Mon, Mar 20, 2023 at 12:34:41PM +0200, Menna Mahmoud wrote:
>> Convert `dev_to_sdio_func` macro into a static inline function.
>> it is not great to have macro that use `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@inria.fr>
>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>> ---
>>   drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
>> index 490431484524..7ee821dbbae0 100644
>> --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
>> +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
>> @@ -10,7 +10,10 @@
>>   #include <linux/jiffies.h>
>>   
>>   #ifndef dev_to_sdio_func
>> -#define dev_to_sdio_func(d)     container_of(d, struct sdio_func, dev)
>> +static inline struct sdio_func *dev_to_sdio_func(struct device *d)
>> +{
>> +	return container_of(d, struct sdio_func, dev);
>> +}
>>   #endif
> Why is the "#ifndef" check still needed now?  Really it was never
> needed, but now would be a great time to remove it as it doubly does not
> make any sense here.
>
> Oh wait, no, this whole thing can just be removed entirely, right?
> There already is a dev_to_sdio_func macro defined, so the #ifndef check
> catches that so your change doesn't actually modify any code that is
> used.  So this should all be removed, not changed to an inline function.

yes, got it.


Thanks,

Menna

>
> thanks,
>
> greg k-h