From: Joe Perches <joe@perches.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>,
Greg KH <gregkh@linuxfoundation.org>
Cc: Arushi Singhal <arushisinghal19971997@gmail.com>,
driverdevel <devel@driverdev.osuosl.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid precedence issues
Date: Wed, 15 Feb 2017 01:31:18 -0800 [thread overview]
Message-ID: <1487151078.6214.50.camel@perches.com> (raw)
In-Reply-To: <CAMuHMdW4Onwp+-eaGDLrqHRqZcHgWjHLRJ1r-gcAh4tYRkNPtQ@mail.gmail.com>
On Wed, 2017-02-15 at 09:14 +0100, Geert Uytterhoeven wrote:
> On Tue, Feb 14, 2017 at 7:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Feb 14, 2017 at 11:26:20PM +0530, Arushi Singhal wrote:
> > > Macro argument 'a' may be better as '(a)' to avoid precedence issues as
> > > reported by checkpatch.pl
> > >
> > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> > > ---
> > > drivers/staging/fwserial/fwserial.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> > > index 41a49c8194e5..bdfc0a8c7af3 100644
> > > --- a/drivers/staging/fwserial/fwserial.c
> > > +++ b/drivers/staging/fwserial/fwserial.c
> > > @@ -98,7 +98,7 @@ struct fwtty_transaction {
> > > };
> > > };
> > >
> > > -#define to_device(a, b) (a->b)
> > > +#define to_device((a), b) (a->b)
> >
> > Really?
>
> Actually compiling files should be a requirement for submitting patches,
> cfr. the response from kbuild test report.
> The parentheses should be added to the second "a" instead ;-)
>
> > Why do we even have this macro at all? Can it just be removed?
>
> Definitely. Would be much easier to read.
The use of & in some of the fwtty_<level> macro uses
suggest it would be easier to read if the fwtty_macros
were removed altogether and simply converted to their
dev_<level> equivalents.
$ git grep -E -w -n "fwtty_(err|info|notice|warn|dbg|err_ratelimited)"| cat
drivers/staging/fwserial/fwserial.c:101:#define fwtty_err(p, fmt, ...) \
drivers/staging/fwserial/fwserial.c:103:#define fwtty_info(p, fmt, ...) \
drivers/staging/fwserial/fwserial.c:105:#define fwtty_notice(p, fmt, ...) \
drivers/staging/fwserial/fwserial.c:107:#define fwtty_dbg(p, fmt, ...) \
drivers/staging/fwserial/fwserial.c:109:#define fwtty_err_ratelimited(p, fmt, ...) \
drivers/staging/fwserial/fwserial.c:121: fwtty_dbg(port, "short write: avail:%d req:%d wrote:%d\n",
drivers/staging/fwserial/fwserial.c:200: fwtty_err_ratelimited(port, "card busy\n");
drivers/staging/fwserial/fwserial.c:203: fwtty_err_ratelimited(port, "bad unit addr or write length\n");
drivers/staging/fwserial/fwserial.c:206: fwtty_err_ratelimited(port, "failed rx\n");
drivers/staging/fwserial/fwserial.c:209: fwtty_err_ratelimited(port, "missing ack\n");
drivers/staging/fwserial/fwserial.c:212: fwtty_err_ratelimited(port, "remote busy\n");
drivers/staging/fwserial/fwserial.c:215: fwtty_err_ratelimited(port, "failed tx: %d\n", rcode);
drivers/staging/fwserial/fwserial.c:289: fwtty_dbg(port, "fifo len: %d avail: %d\n", len, avail);
drivers/staging/fwserial/fwserial.c:326: fwtty_dbg(port, "status: %x delta: %x\n", status, delta);
drivers/staging/fwserial/fwserial.c:522: fwtty_dbg(port, "sending %d brks\n", n);
drivers/staging/fwserial/fwserial.c:546: fwtty_dbg(port, "%d\n", n);
drivers/staging/fwserial/fwserial.c:586: fwtty_err_ratelimited(port, "flip buffer overrun\n");
drivers/staging/fwserial/fwserial.c:629: fwtty_err_ratelimited(port, "ignoring unauthenticated data\n");
drivers/staging/fwserial/fwserial.c:686: fwtty_dbg(port, "rcode: %d\n", rcode);
drivers/staging/fwserial/fwserial.c:749: fwtty_dbg(port, "out: %u rem: %d\n", txn->dma_pended.len, n);
drivers/staging/fwserial/fwserial.c:759: fwtty_err_ratelimited(port, "fifo err: %d\n",
drivers/staging/fwserial/fwserial.c:819: fwtty_dbg(port, "%02x\n", ch);
drivers/staging/fwserial/fwserial.c:904: fwtty_dbg(port, "on/off: %d\n", on);
drivers/staging/fwserial/fwserial.c:932: fwtty_dbg(port, "%d\n", rc);
drivers/staging/fwserial/fwserial.c:1110: fwtty_dbg(port, "%d\n", c);
drivers/staging/fwserial/fwserial.c:1137: fwtty_dbg(port, "%d\n", n);
drivers/staging/fwserial/fwserial.c:1151: fwtty_dbg(port, "%d\n", n);
drivers/staging/fwserial/fwserial.c:1160: fwtty_dbg(port, "%02x\n", ch);
drivers/staging/fwserial/fwserial.c:1187: fwtty_dbg(port, "CRTSCTS: %d\n", C_CRTSCTS(tty) != 0);
drivers/staging/fwserial/fwserial.c:1341: fwtty_dbg(port, "%d\n", state);
drivers/staging/fwserial/fwserial.c:1378: fwtty_dbg(port, "%x\n", tiocm);
drivers/staging/fwserial/fwserial.c:1388: fwtty_dbg(port, "set: %x clear: %x\n", set, clear);
drivers/staging/fwserial/fwserial.c:1700: fwtty_info(&peer->unit, "peer (guid:%016llx) connected on %s\n",
drivers/staging/fwserial/fwserial.c:1722: fwtty_dbg(&peer->unit, "mgmt write error: %d\n", rcode);
drivers/staging/fwserial/fwserial.c:1844: fwtty_err(&peer->unit, "avail ports in use\n");
drivers/staging/fwserial/fwserial.c:1982: fwtty_err(card, "unknown card (guid %016llx)\n",
drivers/staging/fwserial/fwserial.c:2012: fwtty_dbg(card, "peer(%d:%x) guid: %016llx\n",
drivers/staging/fwserial/fwserial.c:2108: fwtty_info(&peer->unit, "peer added (guid:%016llx)\n",
drivers/staging/fwserial/fwserial.c:2172: fwtty_info(&peer->unit, "peer removed (guid:%016llx)\n",
drivers/staging/fwserial/fwserial.c:2259: fwtty_err(&unit, "no space in port table\n");
drivers/staging/fwserial/fwserial.c:2270: fwtty_err(&unit, "register tty device error (%d)\n",
drivers/staging/fwserial/fwserial.c:2288: fwtty_err(&unit, "create loop device failed (%d)\n",
drivers/staging/fwserial/fwserial.c:2309: fwtty_notice(&unit, "TTY over FireWire on device %s (guid %016llx)\n",
drivers/staging/fwserial/fwserial.c:2316: fwtty_err(&unit, "unable to add peer unit device (%d)\n", err);
drivers/staging/fwserial/fwserial.c:2557: fwtty_err(&peer->unit, "no more ports avail\n");
drivers/staging/fwserial/fwserial.c:2599: fwtty_err(&peer->unit, "PLUG_RSP error (%d)\n", rcode);
drivers/staging/fwserial/fwserial.c:2650: fwtty_err(&peer->unit, "UNPLUG_RSP error (%d)\n",
drivers/staging/fwserial/fwserial.c:2685: fwtty_err(&peer->unit, "peer already removed\n");
drivers/staging/fwserial/fwserial.c:2692: fwtty_dbg(&peer->unit, "mgmt: hdr.code: %04hx\n", pkt->hdr.code);
drivers/staging/fwserial/fwserial.c:2697: fwtty_err(&peer->unit, "plug req: busy\n");
drivers/staging/fwserial/fwserial.c:2712: fwtty_notice(&peer->unit, "NACK plug rsp\n");
drivers/staging/fwserial/fwserial.c:2728: fwtty_err(&peer->unit, "unplug req: busy\n");
drivers/staging/fwserial/fwserial.c:2741: fwtty_notice(&peer->unit, "NACK unplug?\n");
drivers/staging/fwserial/fwserial.c:2748: fwtty_err(&peer->unit, "unknown mgmt code %d\n",
drivers/staging/fwserial/fwserial.c:2782: fwtty_dbg(card, "peer(%d:%x) not found\n", generation, source);
next prev parent reply other threads:[~2017-02-15 9:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-14 17:56 [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid precedence issues Arushi Singhal
2017-02-14 18:08 ` Greg KH
2017-02-15 8:14 ` Geert Uytterhoeven
2017-02-15 9:31 ` Joe Perches [this message]
2017-02-14 20:27 ` kbuild test robot
2017-02-14 21:17 ` kbuild test robot
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=1487151078.6214.50.camel@perches.com \
--to=joe@perches.com \
--cc=arushisinghal19971997@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
/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