Macros with multiple statements should be enclosed in a do - while block.
Signed-off-by: Husni Faiz ahamedhusni73@gmail.com --- 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, \
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 ahamedhusni73@gmail.com
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 gregkh@linuxfoundation.org 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 gregkh@linuxfoundation.org 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 gregkh@linuxfoundation.org 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 -- greybus-dev@lists.linaro.org To unsubscribe send an email to greybus-dev-leave@lists.linaro.org
-
Ahamed Husni -
Alex Elder -
Greg KH -
Husni Faiz