From: Alex Elder <elder@ieee.org>
To: Dileep Sankhla <dileepsankhla.ds@gmail.com>,
Greg KH <gregkh@linuxfoundation.org>
Cc: greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org, pure.logic@nexus-software.ie,
johan@kernel.org, elder@kernel.org
Subject: Re: [PATCH] staging: greybus: put macro in a do - while loop
Date: Mon, 4 Mar 2024 11:59:13 -0600 [thread overview]
Message-ID: <b6cee7a2-d702-4248-977e-25a91c210c93@ieee.org> (raw)
In-Reply-To: <CAHxc4bsFj1=VFVDWbdwo3W3CmSyPG1585p2zBePpsD9qy6VKdA@mail.gmail.com>
On 2/25/24 3:49 AM, Dileep Sankhla wrote:
> On Sun, Feb 25, 2024 at 2:26 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>> Did you test build this?
>
> Hello Greg,
>
> Yes. No new warning/error was encountered on building the kernel.
Then your build must not have been compiling your changed
code, because the result of your change produces code that
will not compile successfully.
If you look at where gb_loopback_stats_attrs() is called, it's
used only at outer scope, in "drivers/staging/greybus/loopback.c".
Adding do { ... } while() at outer scope is nonsensical.
>
>>> #define gb_loopback_attr(field, type) \
>>> static ssize_t field##_show(struct device *dev, \
>>
>> Why did you only change one if you thought this was a valid change?
>
> 1. As per my C background, I think no other macros in the above source
> code file need to be enclosed in a do - while loop.
gb_loopback_stats_attrs() must *not* be enclosed in a do..while loop.
> 2. I am writing the patch because of the Eudyptula Challenge, and I
> have to fix "one coding style problem" in any of the files in
> drivers/staging/. The above one was one of them.
I support the challenge. But you need to be sure your fix actually
works, and in particular (in this case) that it compiles correctly.
-Alex
>
> Regards,
> Dileep
prev parent reply other threads:[~2024-03-04 17:59 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-25 8:40 [PATCH] staging: greybus: put macro in a do - while loop Dileep Sankhla
2024-02-25 8:56 ` Greg KH
2024-02-25 9:49 ` Dileep Sankhla
2024-03-04 17:59 ` Alex Elder [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b6cee7a2-d702-4248-977e-25a91c210c93@ieee.org \
--to=elder@ieee.org \
--cc=dileepsankhla.ds@gmail.com \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=pure.logic@nexus-software.ie \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).