Macros with multiple statements should be enclosed in a do - while
block.
Signed-off-by: Husni Faiz <[email protected]>
---
drivers/staging/greybus/loopback.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 2471448ba42a..33666a49e0a8 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -162,10 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \
} \
static DEVICE_ATTR_RO(name##_avg)
-#define gb_loopback_stats_attrs(field) \
- gb_loopback_ro_stats_attr(field, min, u); \
- gb_loopback_ro_stats_attr(field, max, u); \
- gb_loopback_ro_avg_attr(field)
+#define gb_loopback_stats_attrs(field) \
+ do { \
+ gb_loopback_ro_stats_attr(field, min, u); \
+ gb_loopback_ro_stats_attr(field, max, u); \
+ gb_loopback_ro_avg_attr(field) \
+ } while (0)
#define gb_loopback_attr(field, type) \
static ssize_t field##_show(struct device *dev, \
--
2.25.1
On Fri, Feb 18, 2022 at 12:37:22AM +0530, Husni Faiz wrote:
> Macros with multiple statements should be enclosed in a do - while
> block.
>
> Signed-off-by: Husni Faiz <[email protected]>
> ---
> drivers/staging/greybus/loopback.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 2471448ba42a..33666a49e0a8 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -162,10 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \
> } \
> static DEVICE_ATTR_RO(name##_avg)
>
> -#define gb_loopback_stats_attrs(field) \
> - gb_loopback_ro_stats_attr(field, min, u); \
> - gb_loopback_ro_stats_attr(field, max, u); \
> - gb_loopback_ro_avg_attr(field)
> +#define gb_loopback_stats_attrs(field) \
> + do { \
> + gb_loopback_ro_stats_attr(field, min, u); \
> + gb_loopback_ro_stats_attr(field, max, u); \
> + gb_loopback_ro_avg_attr(field) \
> + } while (0)
>
> #define gb_loopback_attr(field, type) \
> static ssize_t field##_show(struct device *dev, \
> --
> 2.25.1
>
>
Did you try to build this change?
Hi Greg,
On Fri, Feb 18, 2022 at 12:56 AM Greg KH <[email protected]> wrote:
> Did you try to build this change?
I am a newbie kernel dev and trying to understand how things work.
I did not build this change by the time I sent you this, thinking this
is just a style change.
I should have tested the build. I am sorry.
Now I built the changes by setting the following configurations.
CONFIG_GREYBUS
CONFIG_STAGING
CONFIG_GREYBUS_LOOPBACK
My change introduces the following error.
''''
drivers/staging/greybus/loopback.c:166:2: error: expected identifier
or ‘(’ before ‘do’
166 | do { \
| ^~
''''
I could not fix or find the reason for this error. Please guide me in
this regard.
Thanks,
Husni.
On Fri, Feb 18, 2022 at 10:22:28AM +0530, Ahamed Husni wrote:
> Hi Greg,
>
> On Fri, Feb 18, 2022 at 12:56 AM Greg KH <[email protected]> wrote:
> > Did you try to build this change?
>
> I am a newbie kernel dev and trying to understand how things work.
> I did not build this change by the time I sent you this, thinking this
> is just a style change.
> I should have tested the build. I am sorry.
You always have to build-test your changes, as you have found out.
> Now I built the changes by setting the following configurations.
> CONFIG_GREYBUS
> CONFIG_STAGING
> CONFIG_GREYBUS_LOOPBACK
>
> My change introduces the following error.
> ''''
> drivers/staging/greybus/loopback.c:166:2: error: expected identifier
> or ‘(’ before ‘do’
> 166 | do { \
> | ^~
> ''''
> I could not fix or find the reason for this error. Please guide me in
> this regard.
There is nothing wrong with the original code here. Remember that
checkpatch is a perl script that gives good advice, but it is not always
correct. You must always manually check it based on your knowledge of
the C language.
I recommend learning a bit more C before working on kernel code.
Best of luck!
greg k-h
On 2/17/22 10:52 PM, Ahamed Husni wrote:
> Hi Greg,
>
> On Fri, Feb 18, 2022 at 12:56 AM Greg KH <[email protected]> wrote:
>> Did you try to build this change?
>
> I am a newbie kernel dev and trying to understand how things work.
> I did not build this change by the time I sent you this, thinking this
> is just a style change.
> I should have tested the build. I am sorry.
>
> Now I built the changes by setting the following configurations.
> CONFIG_GREYBUS
> CONFIG_STAGING
> CONFIG_GREYBUS_LOOPBACK
>
> My change introduces the following error.
> ''''
> drivers/staging/greybus/loopback.c:166:2: error: expected identifier
> or ‘(’ before ‘do’
> 166 | do { \
> | ^~
> ''''
> I could not fix or find the reason for this error. Please guide me in
> this regard.
You should understand that you cannot contribute to the Linux
kernel if you don't understand details of the C language well.
And you really must test your changes (certainly a build test)
before you send them for review.
To answer your question, the macro you changed does not
expand into code that is itself incorporated in an executable
block. The macro is used to generate entire functions in a
unified way.
-Alex
>
> Thanks,
> Husni.
> _______________________________________________
> greybus-dev mailing list -- [email protected]
> To unsubscribe send an email to [email protected]