Fix a blank line after function/struct/union/enum declarations. Also, convert to_gb_control() macro into an inline function in order to maintain Linux kernel coding style based on which the inline function is preferable over the macro.
Signed-off-by: Madhumitha Prabakaran madhumithabiw@gmail.com --- drivers/staging/greybus/control.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/control.h b/drivers/staging/greybus/control.h index 3a29ec05f631..a681ef74e7fe 100644 --- a/drivers/staging/greybus/control.h +++ b/drivers/staging/greybus/control.h @@ -24,7 +24,11 @@ struct gb_control { char *vendor_string; char *product_string; }; -#define to_gb_control(d) container_of(d, struct gb_control, dev) + +static inline struct gb_control *to_gb_control(struct device *d) +{ + return container_of(d, struct gb_control, dev); +}
struct gb_control *gb_control_create(struct gb_interface *intf); int gb_control_enable(struct gb_control *control);
On Fri, Apr 05, 2019 at 03:14:37PM -0500, Madhumitha Prabakaran wrote:
Fix a blank line after function/struct/union/enum declarations. Also, convert to_gb_control() macro into an inline function in order to maintain Linux kernel coding style based on which the inline function is preferable over the macro.
There are about 1200 macros wrapping container_of() in the kernel, so this is a common pattern which does not need to be changed (by contrast, only a handful container_of are wrapped by functions).
Signed-off-by: Madhumitha Prabakaran madhumithabiw@gmail.com
drivers/staging/greybus/control.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/control.h b/drivers/staging/greybus/control.h index 3a29ec05f631..a681ef74e7fe 100644 --- a/drivers/staging/greybus/control.h +++ b/drivers/staging/greybus/control.h @@ -24,7 +24,11 @@ struct gb_control { char *vendor_string; char *product_string; }; -#define to_gb_control(d) container_of(d, struct gb_control, dev)
No need for a blank line either as the macro is closely related to the preceding struct declaration.
+static inline struct gb_control *to_gb_control(struct device *d) +{
- return container_of(d, struct gb_control, dev);
+}
Johan
On Mon, Apr 15, 2019 at 03:10:02PM +0200, Johan Hovold wrote:
On Fri, Apr 05, 2019 at 03:14:37PM -0500, Madhumitha Prabakaran wrote:
Fix a blank line after function/struct/union/enum declarations. Also, convert to_gb_control() macro into an inline function in order to maintain Linux kernel coding style based on which the inline function is preferable over the macro.
There are about 1200 macros wrapping container_of() in the kernel, so this is a common pattern which does not need to be changed (by contrast, only a handful container_of are wrapped by functions).
A handful?
$ git grep "return container_of" | wc -l 1856
I'm like Donald Trump because I must have really small hands compared to the rest of y'all.
I would prefer to apply these sorts of patches unless it causes an issue... Functions *are* better than macros. It's a minor improvement, but this is staging and we make tons minor style changes. It's part of the blessing and curse of being in staging...
regards, dan carpenter
On Mon, Apr 15, 2019 at 04:33:57PM +0300, Dan Carpenter wrote:
On Mon, Apr 15, 2019 at 03:10:02PM +0200, Johan Hovold wrote:
On Fri, Apr 05, 2019 at 03:14:37PM -0500, Madhumitha Prabakaran wrote:
Fix a blank line after function/struct/union/enum declarations. Also, convert to_gb_control() macro into an inline function in order to maintain Linux kernel coding style based on which the inline function is preferable over the macro.
There are about 1200 macros wrapping container_of() in the kernel, so this is a common pattern which does not need to be changed (by contrast, only a handful container_of are wrapped by functions).
A handful?
$ git grep "return container_of" | wc -l 1856
I'm like Donald Trump because I must have really small hands compared to the rest of y'all.
Heh, you're right, my grep-fu failed me.
Ok, so both styles are roughly as common (and a non-trivial portion of those 1800 also are not simple wrappers).
I would prefer to apply these sorts of patches unless it causes an issue... Functions *are* better than macros. It's a minor improvement, but this is staging and we make tons minor style changes. It's part of the blessing and curse of being in staging...
While I agree functions are generally preferred, container_of() is special as it includes type checking.
And again, using container_of() like this is a common pattern in the kernel.
Johan
What I'm saying is that if we just apply it then we avoid the long discussion forever. The macro is OK, sure, but it's not like anyone is going to come back later and argue that macros are better or preferred.
regards, dan carpenter
On Mon, Apr 15, 2019 at 05:40:16PM +0300, Dan Carpenter wrote:
What I'm saying is that if we just apply it then we avoid the long discussion forever. The macro is OK, sure, but it's not like anyone is going to come back later and argue that macros are better or preferred.
That may be a valid argument, at least with respect to staging currently (even if checkpatch isn't entirely too blame for this particular "cleanup").
But then all eight or so instances should be replaced in one go, to maintain consistency.
Johan
-
Dan Carpenter -
Johan Hovold -
Madhumitha Prabakaran