The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
This patch modifies the `gb_loopback_stats_attrs` macro to enclose its statements in a do-while loop, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com --- drivers/staging/greybus/loopback.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4313d3bbc23a..a97de86c67a5 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -163,9 +163,11 @@ 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) + 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 Wed, Jul 17, 2024 at 01:58:34AM -0500, Juan José Arboleda wrote:
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
This patch modifies the `gb_loopback_stats_attrs` macro to enclose its statements in a do-while loop, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com
drivers/staging/greybus/loopback.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4313d3bbc23a..a97de86c67a5 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -163,9 +163,11 @@ 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)
- 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, \
Are you sure you test-built this change?
thanks,
greg k-h
Yes! I build and clean and build and clean and build again, everything seems fine from the compiler side and posttest task as well. Everything seems ok.
Thanks, - Juan José
On Wed, Jul 17, 2024 at 2:50 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 17, 2024 at 01:58:34AM -0500, Juan José Arboleda wrote:
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
This patch modifies the `gb_loopback_stats_attrs` macro to enclose its statements in a do-while loop, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com
drivers/staging/greybus/loopback.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c
b/drivers/staging/greybus/loopback.c
index 4313d3bbc23a..a97de86c67a5 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -163,9 +163,11 @@ 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)
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,
\Are you sure you test-built this change?
thanks,
greg k-h
On Wed, Jul 17, 2024 at 09:50:52AM +0200, Greg KH wrote:
On Wed, Jul 17, 2024 at 01:58:34AM -0500, Juan José Arboleda wrote:
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
This patch modifies the `gb_loopback_stats_attrs` macro to enclose its statements in a do-while loop, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com
drivers/staging/greybus/loopback.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4313d3bbc23a..a97de86c67a5 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -163,9 +163,11 @@ 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)
- 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, \
Are you sure you test-built this change?
thanks,
greg k-h
Yes! I build and clean and build and clean and build again, everything seems fine from the compiler side and posttest task as well. Everything seems ok.
Thanks, - Juan José
On Wed, Jul 17, 2024 at 03:28:33AM -0500, Juan José Arboleda wrote:
On Wed, Jul 17, 2024 at 09:50:52AM +0200, Greg KH wrote:
On Wed, Jul 17, 2024 at 01:58:34AM -0500, Juan José Arboleda wrote:
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
This patch modifies the `gb_loopback_stats_attrs` macro to enclose its statements in a do-while loop, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com
drivers/staging/greybus/loopback.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4313d3bbc23a..a97de86c67a5 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -163,9 +163,11 @@ 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)
- 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, \
Are you sure you test-built this change?
thanks,
greg k-h
Yes! I build and clean and build and clean and build again, everything seems fine from the compiler side and posttest task as well. Everything seems ok.
You did not actually build this file that you modified :(
Always test-build your changes.
thanks,
greg k-h
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
The latter is not possible as the macro expansion will create invalid C syntax. Theres is not such thing like inner function definition inside a global scope do-while in C.
This patch nukes the `gb_loopback_stats_attrs` macro to enclose, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com --- Notes:
This new version uses a bit different approach so the commit message will be a bit different as well.
P.S: Thanks for being that patience and apologies for that false positive of building the non-included driver. Now it is tested-built.
drivers/staging/greybus/loopback.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4313d3bbc23a..88d86b084f28 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -162,11 +162,6 @@ 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_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ @@ -268,15 +263,29 @@ static void gb_loopback_check_attr(struct gb_loopback *gb) }
/* Time to send and receive one message */ -gb_loopback_stats_attrs(latency); +gb_loopback_ro_stats_attr(latency, min, u); +gb_loopback_ro_stats_attr(latency, max, u); +gb_loopback_ro_avg_attr(latency); + /* Number of requests sent per second on this cport */ -gb_loopback_stats_attrs(requests_per_second); +gb_loopback_ro_stats_attr(requests_per_second, min, u); +gb_loopback_ro_stats_attr(requests_per_second, max, u); +gb_loopback_ro_avg_attr(requests_per_second); + /* Quantity of data sent and received on this cport */ -gb_loopback_stats_attrs(throughput); +gb_loopback_ro_stats_attr(throughput, min, u); +gb_loopback_ro_stats_attr(throughput, max, u); +gb_loopback_ro_avg_attr(throughput); + /* Latency across the UniPro link from APBridge's perspective */ -gb_loopback_stats_attrs(apbridge_unipro_latency); +gb_loopback_ro_stats_attr(apbridge_unipro_latency, min, u); +gb_loopback_ro_stats_attr(apbridge_unipro_latency, max, u); +gb_loopback_ro_avg_attr(apbridge_unipro_latency); + /* Firmware induced overhead in the GPBridge */ -gb_loopback_stats_attrs(gbphy_firmware_latency); +gb_loopback_ro_stats_attr(gbphy_firmware_latency, min, u); +gb_loopback_ro_stats_attr(gbphy_firmware_latency, max, u); +gb_loopback_ro_avg_attr(gbphy_firmware_latency);
/* Number of errors encountered during loop */ gb_loopback_ro_attr(error);
On Wed, Jul 17, 2024 at 10:32:07PM -0500, Juan José Arboleda wrote:
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
The latter is not possible as the macro expansion will create invalid C syntax. Theres is not such thing like inner function definition inside a global scope do-while in C.
This patch nukes the `gb_loopback_stats_attrs` macro to enclose, addressing the style error flagged by checkpatch.pl.
The style error here is just wrong, the original code is fine. Remember, checkpatch provides hints, it isn't always correct, and many times, like this one, it's completely incorrect.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com
Notes:
This new version uses a bit different approach so the commit message will be a bit different as well.
P.S: Thanks for being that patience and apologies for that false positive of building the non-included driver. Now it is tested-built.
drivers/staging/greybus/loopback.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4313d3bbc23a..88d86b084f28 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -162,11 +162,6 @@ 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_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ @@ -268,15 +263,29 @@ static void gb_loopback_check_attr(struct gb_loopback *gb) } /* Time to send and receive one message */ -gb_loopback_stats_attrs(latency); +gb_loopback_ro_stats_attr(latency, min, u); +gb_loopback_ro_stats_attr(latency, max, u); +gb_loopback_ro_avg_attr(latency);
Unrolling the macro like this does fix the warning, but at the expense of more code size and manual work. We write code first for us to understand, and second for the compiler. The original code is just fine, no need for a change at all.
sorry,
greg k-h
On Wed, Jul 17, 2024 at 10:32:07PM -0500, Juan José Arboleda wrote:
The macro `gb_loopback_stats_attrs` in the Greybus loopback driver contains multiple statements, which should be enclosed in a do-while loop to ensure proper execution and adhere to coding standards.
The latter is not possible as the macro expansion will create invalid C syntax. Theres is not such thing like inner function definition inside a global scope do-while in C.
This patch nukes the `gb_loopback_stats_attrs` macro to enclose, addressing the style error flagged by checkpatch.pl.
Signed-off-by: Juan José Arboleda soyjuanarbol@gmail.com
Sorry, it's not really important to follow checkpatch. Checkpatch is just a Perl script and sometimes it gets things wrong. It's a tool not a king.
regards, dan carpenter
-
Dan Carpenter -
Greg KH -
Juan José -
Juan José Arboleda