Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580719 > unrolled thread

[PATCH] staging: fwserial: replace 'a' with '(a)' to avoid precedence issues

Started byArushi Singhal <arushisinghal19971997@gmail.com>
First post2017-02-14 19:00 +0100
Last post2017-02-15 10:40 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid  precedence issues Arushi Singhal <arushisinghal19971997@gmail.com> - 2017-02-14 19:00 +0100
    Re: [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid  precedence issues Greg KH <gregkh@linuxfoundation.org> - 2017-02-14 19:10 +0100
      Re: [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid  precedence issues Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-15 09:20 +0100
        Re: [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid  precedence issues Joe Perches <joe@perches.com> - 2017-02-15 10:40 +0100

#1580719 — [PATCH] staging: fwserial: replace 'a' with '(a)' to avoid precedence issues

FromArushi Singhal <arushisinghal19971997@gmail.com>
Date2017-02-14 19:00 +0100
Subject[PATCH] staging: fwserial: replace 'a' with '(a)' to avoid precedence issues
Message-ID<taPyp-4RU-11@gated-at.bofh.it>
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)
 #define fwtty_err(p, fmt, ...)						\
 	dev_err(to_device(p, device), fmt, ##__VA_ARGS__)
 #define fwtty_info(p, fmt, ...)						\
-- 
2.11.0

[toc] | [next] | [standalone]

#1580725

FromGreg KH <gregkh@linuxfoundation.org>
Date2017-02-14 19:10 +0100
Message-ID<taPI5-5ax-15@gated-at.bofh.it>
In reply to#1580719
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?  Why do we even have this macro at all?  Can it just be removed?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1581111

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-02-15 09:20 +0100
Message-ID<tb2YG-5LQ-15@gated-at.bofh.it>
In reply to#1580725
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.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1581162

FromJoe Perches <joe@perches.com>
Date2017-02-15 10:40 +0100
Message-ID<tb4e6-6AO-5@gated-at.bofh.it>
In reply to#1581111
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);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web