Re: [PATCH] staging: greybus: eclose macro in a do - while loop

From: Menna Mahmoud
Date: Sun Mar 12 2023 - 09:44:16 EST



On ١١‏/٣‏/٢٠٢٣ ٢١:١٠, Alex Elder wrote:
On 3/11/23 7:59 AM, Menna Mahmoud wrote:
" ERROR: Macros with multiple statements should be enclosed in a do -
while loop"

Reported by checkpath.

This is also not an issue that should be "fixed."

If you look at where that macro is expanded, you see
that its purpose is simply to reduce the possibility
of some errors by enclosing some much-duplicated code
in this macro.  The expansion is at the top level of
the source file, so a "do...while" loop ends up being
an error.

When looking at the output of checkpatch, assume it's
giving you clues about problems that one *might* like to
fix.  Its suggestions are most often reasonable, but in
some cases (like this one) it's just not smart enough
to recognize the problem that comes from following its
advice.

Make sure you understand exactly what happens when
you make a change.  That means understanding the
code, and then it means ensuring that the fix passes
at least a compile test, and if possible an actual
execution test.

                    -Alex


I see, Thanks Alex for explaining. I will check the code before making any change.

Menna




do loop with the conditional expression set to a constant
value of zero (0).This creates a loop that
will execute exactly one time.This is a coding idiom that
allows a multi-line macro to be used anywhere
that a single statement can be used.

So, enclose `gb_loopback_stats_attrs` macro in do - while (0) to
fix checkpath error

Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@xxxxxxxxx>
---
  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 1a61fce98056..e86d50638cb5 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, \