Message ID | Y0wS4HQo9m/W/TrQ@debian-BULLSEYE-live-builder-AMD64 |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp1046263wrs; Sun, 16 Oct 2022 07:48:28 -0700 (PDT) X-Google-Smtp-Source: AMsMyM62jNz2YSSvJpaZCL634vnjexevm2XCrETQ2W5q4FOcO/nRf1iGgw15WHtHh3vSMHUCkaLr X-Received: by 2002:a17:902:e88d:b0:185:3ac3:e1fd with SMTP id w13-20020a170902e88d00b001853ac3e1fdmr7410037plg.114.1665931708253; Sun, 16 Oct 2022 07:48:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1665931708; cv=none; d=google.com; s=arc-20160816; b=m9Gebt7Jt6Wnx1KpVd2Sn10sA1P6W1Xybkl+gmwkWioMKYpQKhdtURmEJT52kaLnXO 8TRvGxzq98IpQogQ0/iC+wT3MNLDzk33ZMeSpqRY9y+yiKj4nSks7/3oz3E6QGZzJggu Cn0wNEOoKh4oB/2sDxN6ADYWgW7T55g6/CzWkRj/CdpoZQi9kUBSVC0uAwZdbLHynBxG cOh9QiDtJ7OlWg8ehU4ltYiriMurAhJnQmcEFLrB29iHEE8WkSmt3eUtr4IhVc3PiAXX bprgEXZT8D1wJ9s0EfQzHoEebaPSENSpBriWSONkDG5+eekprbes5LvvpoYebXzxqIfz d1ow== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-disposition:mime-version:message-id :subject:to:from:date:dkim-signature; bh=WO+k1977z0WvCv5GUpSkaOkuSb5Iv70KTYS+Dl4CO0o=; b=IDqsYxI1z/aqKsMC+VbQkcWro7XQiAgC8z5l8ja+j2vxT4c8YrZcIXLw6so8OaWxIL ao83HDwdm2bOtdOO8h78CC/DFdg4/+ScMf6dQ51MA1M1qr55o7BxLmQMCSXJOg/QNveE Xn3855vs8tZjEk17Bp7wcQ7ajrrlfLdJAATxP7clgnazFtAkMfbL5Te2zWI7MoSnQLYO s/PrbEyjVd6nLTRyPZb+I8xQ5T2yBdAQdG0VyGb/RSLIal9kTrXnfgw7MfDkWlIZfS/N C2843uJvJ6K9j+M6pPEA4ouebiC+HP+1tzYojqoLEozm2HVnZQzKXNgel0Gc0EFJK6bk 05mg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@mailo.com header.s=mailo header.b=Jr7gAm6M; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id o4-20020a634104000000b0043a0510a99dsi8729089pga.515.2022.10.16.07.48.15; Sun, 16 Oct 2022 07:48:28 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@mailo.com header.s=mailo header.b=Jr7gAm6M; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229616AbiJPOkK (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Sun, 16 Oct 2022 10:40:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43026 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229577AbiJPOkI (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 16 Oct 2022 10:40:08 -0400 X-Greylist: delayed 1221 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Sun, 16 Oct 2022 07:40:05 PDT Received: from msg-1.mailo.com (msg-1.mailo.com [213.182.54.11]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 848D831376 for <linux-kernel@vger.kernel.org>; Sun, 16 Oct 2022 07:40:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=mailo.com; s=mailo; t=1665929957; bh=eGiDjkHU2H18+UPJbCBnMLwPn0ooEAda11HpwCTW7NE=; h=X-EA-Auth:Date:From:To:Subject:Message-ID:MIME-Version: Content-Type; b=Jr7gAm6Ms40j66UKi9lALGQDZMf4kYHTU3PElDk+o5p3llOSr+G5X5a+Ubi6+7Y2v wb0sLR1w7WpmjPJdZxHX2Ov+Wcpb8mgAcUK0BqGOaFe0mxx8coqzEgfLiLaQFFtyFI h7fFNPbqcSX5sDcs8XHR4BMkGSYnFvnSCFT9/RLM= Received: by b-5.in.mailobj.net [192.168.90.15] with ESMTP via [213.182.55.206] Sun, 16 Oct 2022 16:19:16 +0200 (CEST) X-EA-Auth: ZlYqw7q+n9bWm0E6M2C4cko2RDVMvIgADjfKHRVLOR30Cm/e+LuD7Xdw2nHPPvVfc+puoJzaiDklCQrDpRBLDd7molkJY2vr Date: Sun, 16 Oct 2022 10:19:12 -0400 From: Deepak R Varma <drv@mailo.com> To: outreachy@lists.linux.dev, pure.logic@nexus-software.ie, johan@kernel.org, elder@kernel.org, gregkh@linuxfoundation.org, greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [PATCH] staging: greybus: loopback: enclose macro statements in do-while loop Message-ID: <Y0wS4HQo9m/W/TrQ@debian-BULLSEYE-live-builder-AMD64> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1746856007054423588?= X-GMAIL-MSGID: =?utf-8?q?1746856007054423588?= |
Series |
staging: greybus: loopback: enclose macro statements in do-while loop
|
|
Commit Message
Deepak R Varma
Oct. 16, 2022, 2:19 p.m. UTC
Include multiple statements of macro definition inside do-while{0} loop
to avoid possible partial program execution. Issue reported by
checkpatch script:
ERROR: Macros with multiple statements should be enclosed in a do - while loop
Signed-off-by: Deepak R Varma <drv@mailo.com>
---
drivers/staging/greybus/loopback.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--
2.30.2
Comments
On Sun, 16 Oct 2022, Deepak R Varma wrote: > Include multiple statements of macro definition inside do-while{0} loop > to avoid possible partial program execution. Issue reported by > checkpatch script: > > ERROR: Macros with multiple statements should be enclosed in a do - while loop I don't think this change will compile. See if you can figure out why not. julia > > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > drivers/staging/greybus/loopback.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > index 1a61fce98056..37214cb43937 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) \ > +do { \ > gb_loopback_ro_stats_attr(field, min, u); \ > gb_loopback_ro_stats_attr(field, max, u); \ > - gb_loopback_ro_avg_attr(field) > + gb_loopback_ro_avg_attr(field); \ > +} while (0) > > #define gb_loopback_attr(field, type) \ > static ssize_t field##_show(struct device *dev, \ > -- > 2.30.2 > > > > >
On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > Include multiple statements of macro definition inside do-while{0} loop > > to avoid possible partial program execution. Issue reported by > > checkpatch script: > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > I don't think this change will compile. See if you can figure out why > not. It did compile. I built the greybus driver and loaded it as well with the modinfo tool. Can you please tell why you think it won't compile? ./drv > > julia > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > --- > > drivers/staging/greybus/loopback.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > > index 1a61fce98056..37214cb43937 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) \ > > +do { \ > > gb_loopback_ro_stats_attr(field, min, u); \ > > gb_loopback_ro_stats_attr(field, max, u); \ > > - gb_loopback_ro_avg_attr(field) > > + gb_loopback_ro_avg_attr(field); \ > > +} while (0) > > > > #define gb_loopback_attr(field, type) \ > > static ssize_t field##_show(struct device *dev, \ > > -- > > 2.30.2 > > > > > > > > > >
On Sun, Oct 16, 2022 at 10:19:12AM -0400, Deepak R Varma wrote: > Include multiple statements of macro definition inside do-while{0} loop > to avoid possible partial program execution. Issue reported by > checkpatch script: > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > drivers/staging/greybus/loopback.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > index 1a61fce98056..37214cb43937 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) \ > +do { \ > gb_loopback_ro_stats_attr(field, min, u); \ > gb_loopback_ro_stats_attr(field, max, u); \ > - gb_loopback_ro_avg_attr(field) > + gb_loopback_ro_avg_attr(field); \ > +} while (0) > > #define gb_loopback_attr(field, type) \ > static ssize_t field##_show(struct device *dev, \ > -- > 2.30.2 Always test-build your changes before sending them out so you do not get grumpy maintainers asking why you did not test-build your changes. Also, don't bindly trust that checkpatch is always correct, you need to read the C code to verify that it is a sane request. thanks, greg k-h
On Sun, 16 Oct 2022, Deepak R Varma wrote: > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > to avoid possible partial program execution. Issue reported by > > > checkpatch script: > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > I don't think this change will compile. See if you can figure out why > > not. > > It did compile. I built the greybus driver and loaded it as well with the > modinfo tool. Can you please tell why you think it won't compile? Do you have a .o file for the .c file that you changed? julia > > ./drv > > > > > julia > > > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > --- > > > drivers/staging/greybus/loopback.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > > > index 1a61fce98056..37214cb43937 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) \ > > > +do { \ > > > gb_loopback_ro_stats_attr(field, min, u); \ > > > gb_loopback_ro_stats_attr(field, max, u); \ > > > - gb_loopback_ro_avg_attr(field) > > > + gb_loopback_ro_avg_attr(field); \ > > > +} while (0) > > > > > > #define gb_loopback_attr(field, type) \ > > > static ssize_t field##_show(struct device *dev, \ > > > -- > > > 2.30.2 > > > > > > > > > > > > > > > > > >
On Sun, Oct 16, 2022 at 05:10:17PM +0200, Julia Lawall wrote: > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > > to avoid possible partial program execution. Issue reported by > > > > checkpatch script: > > > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > > > I don't think this change will compile. See if you can figure out why > > > not. > > > > It did compile. I built the greybus driver and loaded it as well with the > > modinfo tool. Can you please tell why you think it won't compile? > > Do you have a .o file for the .c file that you changed? I see many .o files and a greybus.ko as well, but not the loopback.o Am I missing anything with my configuration? I did set Greybus Support to (M) in the menuconfig. Thank you, ./drv > > julia > > > > > ./drv > > > > > > > > julia > > > > > > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > > --- > > > > drivers/staging/greybus/loopback.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > > > > index 1a61fce98056..37214cb43937 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) \ > > > > +do { \ > > > > gb_loopback_ro_stats_attr(field, min, u); \ > > > > gb_loopback_ro_stats_attr(field, max, u); \ > > > > - gb_loopback_ro_avg_attr(field) > > > > + gb_loopback_ro_avg_attr(field); \ > > > > +} while (0) > > > > > > > > #define gb_loopback_attr(field, type) \ > > > > static ssize_t field##_show(struct device *dev, \ > > > > -- > > > > 2.30.2 > > > > > > > > > > > > > > > > > > > > > > > > > > >
On Sun, 16 Oct 2022, Deepak R Varma wrote: > On Sun, Oct 16, 2022 at 05:10:17PM +0200, Julia Lawall wrote: > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > > > to avoid possible partial program execution. Issue reported by > > > > > checkpatch script: > > > > > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > > > > > I don't think this change will compile. See if you can figure out why > > > > not. > > > > > > It did compile. I built the greybus driver and loaded it as well with the > > > modinfo tool. Can you please tell why you think it won't compile? > > > > Do you have a .o file for the .c file that you changed? > > I see many .o files and a greybus.ko as well, but not the loopback.o > Am I missing anything with my configuration? I did set Greybus Support to (M) in > the menuconfig. Something must be missing in the configuration. With make allyesconfig, you can just compile the file you changed, eg make drivers/staging/greybus/loopback.o and see if just that file compiles. Sometimes you can's compile an individual file. In that case, it may be possible to do make linux/file/path/ (assuming your file is in linux/file/path/foo.c). The trailing / is essential. make linux/file/path will do nothing. julia > > Thank you, > ./drv > > > > > julia > > > > > > > > ./drv > > > > > > > > > > > julia > > > > > > > > > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > > > --- > > > > > drivers/staging/greybus/loopback.c | 4 +++- > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > > > > > index 1a61fce98056..37214cb43937 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) \ > > > > > +do { \ > > > > > gb_loopback_ro_stats_attr(field, min, u); \ > > > > > gb_loopback_ro_stats_attr(field, max, u); \ > > > > > - gb_loopback_ro_avg_attr(field) > > > > > + gb_loopback_ro_avg_attr(field); \ > > > > > +} while (0) > > > > > > > > > > #define gb_loopback_attr(field, type) \ > > > > > static ssize_t field##_show(struct device *dev, \ > > > > > -- > > > > > 2.30.2 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
On Sun, Oct 16, 2022 at 11:27:30AM -0400, Deepak R Varma wrote: > On Sun, Oct 16, 2022 at 05:10:17PM +0200, Julia Lawall wrote: > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > > > to avoid possible partial program execution. Issue reported by > > > > > checkpatch script: > > > > > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > > > > > I don't think this change will compile. See if you can figure out why > > > > not. > > > > > > It did compile. I built the greybus driver and loaded it as well with the > > > modinfo tool. Can you please tell why you think it won't compile? > > > > Do you have a .o file for the .c file that you changed? > > I see many .o files and a greybus.ko as well, but not the loopback.o > Am I missing anything with my configuration? I did set Greybus Support to (M) in > the menuconfig. CONFIG_GREYBUS_LOOPBACK has to be enabled in order to build the drivers/staging/greybus/loopback.c file. A simple check would be to do: make drivers/staging/greybus/loopback.o does that work with your change? thanks, greg k-h
On Sun, Oct 16, 2022 at 05:40:59PM +0200, Greg KH wrote: > On Sun, Oct 16, 2022 at 11:27:30AM -0400, Deepak R Varma wrote: > > On Sun, Oct 16, 2022 at 05:10:17PM +0200, Julia Lawall wrote: > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > > > > to avoid possible partial program execution. Issue reported by > > > > > > checkpatch script: > > > > > > > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > > > > > > > I don't think this change will compile. See if you can figure out why > > > > > not. > > > > > > > > It did compile. I built the greybus driver and loaded it as well with the > > > > modinfo tool. Can you please tell why you think it won't compile? > > > > > > Do you have a .o file for the .c file that you changed? > > > > I see many .o files and a greybus.ko as well, but not the loopback.o > > Am I missing anything with my configuration? I did set Greybus Support to (M) in > > the menuconfig. > > CONFIG_GREYBUS_LOOPBACK has to be enabled in order to build the > drivers/staging/greybus/loopback.c file. > > A simple check would be to do: > make drivers/staging/greybus/loopback.o > > does that work with your change? No, it did not. I understand why it did not. My apologies for not looking into the build of loopback.o file when the greybus module was rebuilt. Please ignore my patch. Thank you Julia and Greg for the feedback. ./drv > > thanks, > > greg k-h >
On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > Include multiple statements of macro definition inside do-while{0} loop > > to avoid possible partial program execution. Issue reported by > > checkpatch script: > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > I don't think this change will compile. See if you can figure out why > not. After trying a direct compile of the loopback.c file, it did not compile. The kernel build ran did not compile the loopback.c file due to missing configuration. About this change, the macro expands to function declarations at compile time and those can't be enclosed in do/while loop as these are not logical execution instructions. So it won't compile. I initially looked for "greybus driver" under the "main menu > drivers > staging drivers" path, but not find any configurations for the driver. While retuning back, I found "greybus support" config under "menu-menu > device drivers" itself. I set it to "M" and build the module. I did not realise that setting this parameter to "M" actually results in adding several configurations under "staging drivers" path. I now understand that. Thank you for your help. I will look for another sensible change and send my new first patch. ./drv > > julia > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > --- > > drivers/staging/greybus/loopback.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c > > index 1a61fce98056..37214cb43937 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) \ > > +do { \ > > gb_loopback_ro_stats_attr(field, min, u); \ > > gb_loopback_ro_stats_attr(field, max, u); \ > > - gb_loopback_ro_avg_attr(field) > > + gb_loopback_ro_avg_attr(field); \ > > +} while (0) > > > > #define gb_loopback_attr(field, type) \ > > static ssize_t field##_show(struct device *dev, \ > > -- > > 2.30.2 > > > > > > > > > >
Hi Deepak, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] url: https://github.com/intel-lab-lkp/linux/commits/Deepak-R-Varma/staging-greybus-loopback-enclose-macro-statements-in-do-while-loop/20221017-120025 patch link: https://lore.kernel.org/r/Y0wS4HQo9m%2FW%2FTrQ%40debian-BULLSEYE-live-builder-AMD64 patch subject: [PATCH] staging: greybus: loopback: enclose macro statements in do-while loop config: sparc64-randconfig-r005-20221017 compiler: sparc64-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/5196dd42760e5aed5f688894b7753a4f70bd097b git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Deepak-R-Varma/staging-greybus-loopback-enclose-macro-statements-in-do-while-loop/20221017-120025 git checkout 5196dd42760e5aed5f688894b7753a4f70bd097b # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc64 SHELL=/bin/bash M=drivers/staging/greybus If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/staging/greybus/loopback.c:166:1: error: expected identifier or '(' before 'do' 166 | do { \ | ^~ drivers/staging/greybus/loopback.c:273:1: note: in expansion of macro 'gb_loopback_stats_attrs' 273 | gb_loopback_stats_attrs(latency); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:170:3: error: expected identifier or '(' before 'while' 170 | } while (0) | ^~~~~ drivers/staging/greybus/loopback.c:273:1: note: in expansion of macro 'gb_loopback_stats_attrs' 273 | gb_loopback_stats_attrs(latency); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:166:1: error: expected identifier or '(' before 'do' 166 | do { \ | ^~ drivers/staging/greybus/loopback.c:275:1: note: in expansion of macro 'gb_loopback_stats_attrs' 275 | gb_loopback_stats_attrs(requests_per_second); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:170:3: error: expected identifier or '(' before 'while' 170 | } while (0) | ^~~~~ drivers/staging/greybus/loopback.c:275:1: note: in expansion of macro 'gb_loopback_stats_attrs' 275 | gb_loopback_stats_attrs(requests_per_second); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:166:1: error: expected identifier or '(' before 'do' 166 | do { \ | ^~ drivers/staging/greybus/loopback.c:277:1: note: in expansion of macro 'gb_loopback_stats_attrs' 277 | gb_loopback_stats_attrs(throughput); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:170:3: error: expected identifier or '(' before 'while' 170 | } while (0) | ^~~~~ drivers/staging/greybus/loopback.c:277:1: note: in expansion of macro 'gb_loopback_stats_attrs' 277 | gb_loopback_stats_attrs(throughput); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:166:1: error: expected identifier or '(' before 'do' 166 | do { \ | ^~ drivers/staging/greybus/loopback.c:279:1: note: in expansion of macro 'gb_loopback_stats_attrs' 279 | gb_loopback_stats_attrs(apbridge_unipro_latency); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:170:3: error: expected identifier or '(' before 'while' 170 | } while (0) | ^~~~~ drivers/staging/greybus/loopback.c:279:1: note: in expansion of macro 'gb_loopback_stats_attrs' 279 | gb_loopback_stats_attrs(apbridge_unipro_latency); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:166:1: error: expected identifier or '(' before 'do' 166 | do { \ | ^~ drivers/staging/greybus/loopback.c:281:1: note: in expansion of macro 'gb_loopback_stats_attrs' 281 | gb_loopback_stats_attrs(gbphy_firmware_latency); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:170:3: error: expected identifier or '(' before 'while' 170 | } while (0) | ^~~~~ drivers/staging/greybus/loopback.c:281:1: note: in expansion of macro 'gb_loopback_stats_attrs' 281 | gb_loopback_stats_attrs(gbphy_firmware_latency); | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:319:10: error: 'dev_attr_latency_min' undeclared here (not in a function); did you mean 'dev_attr_timeout_min'? 319 | &dev_attr_latency_min.attr, | ^~~~~~~~~~~~~~~~~~~~ | dev_attr_timeout_min >> drivers/staging/greybus/loopback.c:320:10: error: 'dev_attr_latency_max' undeclared here (not in a function); did you mean 'dev_attr_timeout_max'? 320 | &dev_attr_latency_max.attr, | ^~~~~~~~~~~~~~~~~~~~ | dev_attr_timeout_max >> drivers/staging/greybus/loopback.c:321:10: error: 'dev_attr_latency_avg' undeclared here (not in a function) 321 | &dev_attr_latency_avg.attr, | ^~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:322:10: error: 'dev_attr_requests_per_second_min' undeclared here (not in a function) 322 | &dev_attr_requests_per_second_min.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:323:10: error: 'dev_attr_requests_per_second_max' undeclared here (not in a function) 323 | &dev_attr_requests_per_second_max.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:324:10: error: 'dev_attr_requests_per_second_avg' undeclared here (not in a function) 324 | &dev_attr_requests_per_second_avg.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:325:10: error: 'dev_attr_throughput_min' undeclared here (not in a function); did you mean 'dev_attr_timeout_min'? 325 | &dev_attr_throughput_min.attr, | ^~~~~~~~~~~~~~~~~~~~~~~ | dev_attr_timeout_min >> drivers/staging/greybus/loopback.c:326:10: error: 'dev_attr_throughput_max' undeclared here (not in a function); did you mean 'dev_attr_timeout_max'? 326 | &dev_attr_throughput_max.attr, | ^~~~~~~~~~~~~~~~~~~~~~~ | dev_attr_timeout_max >> drivers/staging/greybus/loopback.c:327:10: error: 'dev_attr_throughput_avg' undeclared here (not in a function) 327 | &dev_attr_throughput_avg.attr, | ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/staging/greybus/loopback.c:328:10: error: 'dev_attr_apbridge_unipro_latency_min' undeclared here (not in a function) 328 | &dev_attr_apbridge_unipro_latency_min.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/staging/greybus/loopback.c:329:10: error: 'dev_attr_apbridge_unipro_latency_max' undeclared here (not in a function) 329 | &dev_attr_apbridge_unipro_latency_max.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/staging/greybus/loopback.c:330:10: error: 'dev_attr_apbridge_unipro_latency_avg' undeclared here (not in a function) 330 | &dev_attr_apbridge_unipro_latency_avg.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/staging/greybus/loopback.c:331:10: error: 'dev_attr_gbphy_firmware_latency_min' undeclared here (not in a function) 331 | &dev_attr_gbphy_firmware_latency_min.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/staging/greybus/loopback.c:332:10: error: 'dev_attr_gbphy_firmware_latency_max' undeclared here (not in a function) 332 | &dev_attr_gbphy_firmware_latency_max.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/staging/greybus/loopback.c:333:10: error: 'dev_attr_gbphy_firmware_latency_avg' undeclared here (not in a function) 333 | &dev_attr_gbphy_firmware_latency_avg.attr, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +166 drivers/staging/greybus/loopback.c 164 165 #define gb_loopback_stats_attrs(field) \ > 166 do { \ 167 gb_loopback_ro_stats_attr(field, min, u); \ 168 gb_loopback_ro_stats_attr(field, max, u); \ 169 gb_loopback_ro_avg_attr(field); \ > 170 } while (0) 171 172 #define gb_loopback_attr(field, type) \ 173 static ssize_t field##_show(struct device *dev, \ 174 struct device_attribute *attr, \ 175 char *buf) \ 176 { \ 177 struct gb_loopback *gb = dev_get_drvdata(dev); \ 178 return sprintf(buf, "%" #type "\n", gb->field); \ 179 } \ 180 static ssize_t field##_store(struct device *dev, \ 181 struct device_attribute *attr, \ 182 const char *buf, \ 183 size_t len) \ 184 { \ 185 int ret; \ 186 struct gb_loopback *gb = dev_get_drvdata(dev); \ 187 mutex_lock(&gb->mutex); \ 188 ret = sscanf(buf, "%"#type, &gb->field); \ 189 if (ret != 1) \ 190 len = -EINVAL; \ 191 else \ 192 gb_loopback_check_attr(gb, bundle); \ 193 mutex_unlock(&gb->mutex); \ 194 return len; \ 195 } \ 196 static DEVICE_ATTR_RW(field) 197 198 #define gb_dev_loopback_ro_attr(field, conn) \ 199 static ssize_t field##_show(struct device *dev, \ 200 struct device_attribute *attr, \ 201 char *buf) \ 202 { \ 203 struct gb_loopback *gb = dev_get_drvdata(dev); \ 204 return sprintf(buf, "%u\n", gb->field); \ 205 } \ 206 static DEVICE_ATTR_RO(field) 207 208 #define gb_dev_loopback_rw_attr(field, type) \ 209 static ssize_t field##_show(struct device *dev, \ 210 struct device_attribute *attr, \ 211 char *buf) \ 212 { \ 213 struct gb_loopback *gb = dev_get_drvdata(dev); \ 214 return sprintf(buf, "%" #type "\n", gb->field); \ 215 } \ 216 static ssize_t field##_store(struct device *dev, \ 217 struct device_attribute *attr, \ 218 const char *buf, \ 219 size_t len) \ 220 { \ 221 int ret; \ 222 struct gb_loopback *gb = dev_get_drvdata(dev); \ 223 mutex_lock(&gb->mutex); \ 224 ret = sscanf(buf, "%"#type, &gb->field); \ 225 if (ret != 1) \ 226 len = -EINVAL; \ 227 else \ 228 gb_loopback_check_attr(gb); \ 229 mutex_unlock(&gb->mutex); \ 230 return len; \ 231 } \ 232 static DEVICE_ATTR_RW(field) 233 234 static void gb_loopback_reset_stats(struct gb_loopback *gb); 235 static void gb_loopback_check_attr(struct gb_loopback *gb) 236 { 237 if (gb->us_wait > GB_LOOPBACK_US_WAIT_MAX) 238 gb->us_wait = GB_LOOPBACK_US_WAIT_MAX; 239 if (gb->size > gb_dev.size_max) 240 gb->size = gb_dev.size_max; 241 gb->requests_timedout = 0; 242 gb->requests_completed = 0; 243 gb->iteration_count = 0; 244 gb->send_count = 0; 245 gb->error = 0; 246 247 if (kfifo_depth < gb->iteration_max) { 248 dev_warn(gb->dev, 249 "cannot log bytes %u kfifo_depth %u\n", 250 gb->iteration_max, kfifo_depth); 251 } 252 kfifo_reset_out(&gb->kfifo_lat); 253 254 switch (gb->type) { 255 case GB_LOOPBACK_TYPE_PING: 256 case GB_LOOPBACK_TYPE_TRANSFER: 257 case GB_LOOPBACK_TYPE_SINK: 258 gb->jiffy_timeout = usecs_to_jiffies(gb->timeout); 259 if (!gb->jiffy_timeout) 260 gb->jiffy_timeout = GB_LOOPBACK_TIMEOUT_MIN; 261 else if (gb->jiffy_timeout > GB_LOOPBACK_TIMEOUT_MAX) 262 gb->jiffy_timeout = GB_LOOPBACK_TIMEOUT_MAX; 263 gb_loopback_reset_stats(gb); 264 wake_up(&gb->wq); 265 break; 266 default: 267 gb->type = 0; 268 break; 269 } 270 } 271 272 /* Time to send and receive one message */ 273 gb_loopback_stats_attrs(latency); 274 /* Number of requests sent per second on this cport */ 275 gb_loopback_stats_attrs(requests_per_second); 276 /* Quantity of data sent and received on this cport */ 277 gb_loopback_stats_attrs(throughput); 278 /* Latency across the UniPro link from APBridge's perspective */ 279 gb_loopback_stats_attrs(apbridge_unipro_latency); 280 /* Firmware induced overhead in the GPBridge */ 281 gb_loopback_stats_attrs(gbphy_firmware_latency); 282 283 /* Number of errors encountered during loop */ 284 gb_loopback_ro_attr(error); 285 /* Number of requests successfully completed async */ 286 gb_loopback_ro_attr(requests_completed); 287 /* Number of requests timed out async */ 288 gb_loopback_ro_attr(requests_timedout); 289 /* Timeout minimum in useconds */ 290 gb_loopback_ro_attr(timeout_min); 291 /* Timeout minimum in useconds */ 292 gb_loopback_ro_attr(timeout_max); 293 294 /* 295 * Type of loopback message to send based on protocol type definitions 296 * 0 => Don't send message 297 * 2 => Send ping message continuously (message without payload) 298 * 3 => Send transfer message continuously (message with payload, 299 * payload returned in response) 300 * 4 => Send a sink message (message with payload, no payload in response) 301 */ 302 gb_dev_loopback_rw_attr(type, d); 303 /* Size of transfer message payload: 0-4096 bytes */ 304 gb_dev_loopback_rw_attr(size, u); 305 /* Time to wait between two messages: 0-1000 ms */ 306 gb_dev_loopback_rw_attr(us_wait, d); 307 /* Maximum iterations for a given operation: 1-(2^32-1), 0 implies infinite */ 308 gb_dev_loopback_rw_attr(iteration_max, u); 309 /* The current index of the for (i = 0; i < iteration_max; i++) loop */ 310 gb_dev_loopback_ro_attr(iteration_count, false); 311 /* A flag to indicate synchronous or asynchronous operations */ 312 gb_dev_loopback_rw_attr(async, u); 313 /* Timeout of an individual asynchronous request */ 314 gb_dev_loopback_rw_attr(timeout, u); 315 /* Maximum number of in-flight operations before back-off */ 316 gb_dev_loopback_rw_attr(outstanding_operations_max, u); 317 318 static struct attribute *loopback_attrs[] = { > 319 &dev_attr_latency_min.attr, > 320 &dev_attr_latency_max.attr, > 321 &dev_attr_latency_avg.attr, > 322 &dev_attr_requests_per_second_min.attr, > 323 &dev_attr_requests_per_second_max.attr, > 324 &dev_attr_requests_per_second_avg.attr, > 325 &dev_attr_throughput_min.attr, > 326 &dev_attr_throughput_max.attr, > 327 &dev_attr_throughput_avg.attr, > 328 &dev_attr_apbridge_unipro_latency_min.attr, > 329 &dev_attr_apbridge_unipro_latency_max.attr, > 330 &dev_attr_apbridge_unipro_latency_avg.attr, > 331 &dev_attr_gbphy_firmware_latency_min.attr, > 332 &dev_attr_gbphy_firmware_latency_max.attr, > 333 &dev_attr_gbphy_firmware_latency_avg.attr, 334 &dev_attr_type.attr, 335 &dev_attr_size.attr, 336 &dev_attr_us_wait.attr, 337 &dev_attr_iteration_count.attr, 338 &dev_attr_iteration_max.attr, 339 &dev_attr_async.attr, 340 &dev_attr_error.attr, 341 &dev_attr_requests_completed.attr, 342 &dev_attr_requests_timedout.attr, 343 &dev_attr_timeout.attr, 344 &dev_attr_outstanding_operations_max.attr, 345 &dev_attr_timeout_min.attr, 346 &dev_attr_timeout_max.attr, 347 NULL, 348 }; 349 ATTRIBUTE_GROUPS(loopback); 350
On Sun, Oct 16, 2022 at 11:50:45AM -0400, Deepak R Varma wrote: > On Sun, Oct 16, 2022 at 05:40:59PM +0200, Greg KH wrote: > > On Sun, Oct 16, 2022 at 11:27:30AM -0400, Deepak R Varma wrote: > > > On Sun, Oct 16, 2022 at 05:10:17PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > > > > > to avoid possible partial program execution. Issue reported by > > > > > > > checkpatch script: > > > > > > > > > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > > > > > > > > > I don't think this change will compile. See if you can figure out why > > > > > > not. > > > > > > > > > > It did compile. I built the greybus driver and loaded it as well with the > > > > > modinfo tool. Can you please tell why you think it won't compile? > > > > > > > > Do you have a .o file for the .c file that you changed? > > > > > > I see many .o files and a greybus.ko as well, but not the loopback.o > > > Am I missing anything with my configuration? I did set Greybus Support to (M) in > > > the menuconfig. > > > > CONFIG_GREYBUS_LOOPBACK has to be enabled in order to build the > > drivers/staging/greybus/loopback.c file. > > > > A simple check would be to do: > > make drivers/staging/greybus/loopback.o > > > > does that work with your change? > > No, it did not. I understand why it did not. My apologies for not looking into > the build of loopback.o file when the greybus module was rebuilt. > > Please ignore my patch. I just received a message from Kernel Test Robot that this patch failed to compile. I had requested to drop/ignore this patch. However, looks like it included in the staging-testing tree???? Let me know anything required from me to fix the bot complaint. Thank you. > > Thank you Julia and Greg for the feedback. > ./drv > > > > > > > thanks, > > > > greg k-h > > > > >
On Wed, Oct 19, 2022 at 08:58:17PM +0530, Deepak R Varma wrote: > On Sun, Oct 16, 2022 at 11:50:45AM -0400, Deepak R Varma wrote: > > On Sun, Oct 16, 2022 at 05:40:59PM +0200, Greg KH wrote: > > > On Sun, Oct 16, 2022 at 11:27:30AM -0400, Deepak R Varma wrote: > > > > On Sun, Oct 16, 2022 at 05:10:17PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > > > On Sun, Oct 16, 2022 at 04:51:09PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > > > > > > > On Sun, 16 Oct 2022, Deepak R Varma wrote: > > > > > > > > > > > > > > > Include multiple statements of macro definition inside do-while{0} loop > > > > > > > > to avoid possible partial program execution. Issue reported by > > > > > > > > checkpatch script: > > > > > > > > > > > > > > > > ERROR: Macros with multiple statements should be enclosed in a do - while loop > > > > > > > > > > > > > > I don't think this change will compile. See if you can figure out why > > > > > > > not. > > > > > > > > > > > > It did compile. I built the greybus driver and loaded it as well with the > > > > > > modinfo tool. Can you please tell why you think it won't compile? > > > > > > > > > > Do you have a .o file for the .c file that you changed? > > > > > > > > I see many .o files and a greybus.ko as well, but not the loopback.o > > > > Am I missing anything with my configuration? I did set Greybus Support to (M) in > > > > the menuconfig. > > > > > > CONFIG_GREYBUS_LOOPBACK has to be enabled in order to build the > > > drivers/staging/greybus/loopback.c file. > > > > > > A simple check would be to do: > > > make drivers/staging/greybus/loopback.o > > > > > > does that work with your change? > > > > No, it did not. I understand why it did not. My apologies for not looking into > > the build of loopback.o file when the greybus module was rebuilt. > > > > Please ignore my patch. > > I just received a message from Kernel Test Robot that this patch failed to > compile. I had requested to drop/ignore this patch. However, looks like it > included in the staging-testing tree???? It just took a while to get to your patch, it picked it up off of the mailing list, the change is not applied anywhere. > Let me know anything required from me to fix the bot complaint. Thank you. There's nothing to do as it's long-gone from my queue. thanks, greg k-h
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 1a61fce98056..37214cb43937 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) \ +do { \ gb_loopback_ro_stats_attr(field, min, u); \ gb_loopback_ro_stats_attr(field, max, u); \ - gb_loopback_ro_avg_attr(field) + gb_loopback_ro_avg_attr(field); \ +} while (0) #define gb_loopback_attr(field, type) \ static ssize_t field##_show(struct device *dev, \