netdev - Re: [PATCH v3] add the driver for Analog Devices Blackfin on-chip CAN controllers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 10 Dec 2009 18:25:49 +0800
From:	Barry Song <21cnbao@...il.com>
To:	Wolfram Sang <w.sang@...gutronix.de>
Cc:	Wolfgang Grandegger <wg@...ndegger.com>,
	socketcan-core@...ts.berlios.de,
	uclinux-dist-devel@...ckfin.uclinux.org, davem@...emloft.net,
	"H.J. Oertel" <oe@...t.de>, netdev@...r.kernel.org
Subject: Re: [PATCH v3] add the driver for Analog Devices Blackfin on-chip CAN 
	controllers

On Thu, Dec 10, 2009 at 6:21 PM, Wolfram Sang <w.sang@...gutronix.de> wrote:
>
>> >> +     if (status) {
>> >> +             priv->can.can_stats.bus_error++;
>> >> +
>> >> +             cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> >> +
>> >> +             if (status & BEF)
>> >> +                     cf->data[2] |= CAN_ERR_PROT_BIT;
>> >> +             else if (status & FER)
>> >> +                     cf->data[2] |= CAN_ERR_PROT_FORM;
>> >> +             else if (status & SER)
>> >> +                     cf->data[2] |= CAN_ERR_PROT_STUFF;
>> >> +             else
>> >> +                     cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>> >> +     }
>> >
>> > Add {} here as well.
>> {} will cause checkpatch warning if it is given to only one line.
>
> Right, no need for braces here, because it is always one statement after
> if/else.
>
>
>> >> +     if ((irq == priv->tx_irq) && readw(&reg->mbtif2)) {
>> >> +             /* transmission complete interrupt */
>> >> +             writew(0xFFFF, &reg->mbtif2);
>> >> +             stats->tx_packets++;
>> >> +             stats->tx_bytes += bfin_can_read_dlc(priv, TRANSMIT_CHL);
>> >> +             can_get_echo_skb(dev, 0);
>> >> +             netif_wake_queue(dev);
>> >> +     } else if ((irq == priv->rx_irq) && readw(&reg->mbrif1)) {
>> >> +             /* receive interrupt */
>> >> +             isrc = readw(&reg->mbrif1);
>> >> +             writew(0xFFFF, &reg->mbrif1);
>> >> +             bfin_can_rx(dev, isrc);
>> >> +     } else if ((irq == priv->err_irq) && readw(&reg->gis)) {
>> >> +             /* error interrupt */
>> >> +             isrc = readw(&reg->gis);
>> >> +             status = readw(&reg->esr);
>> >> +             writew(0x7FF, &reg->gis);
>> >> +             bfin_can_err(dev, isrc, status);
>> >> +     } else
>> >> +             return IRQ_NONE;
>> >
>> > Use {} here as well.
>> {} will cause checkpatch warning if it is given to only one line.
>
> Then again, here braces are suggested. If the 'if'-block has braces, the
> 'else'-block should have braces, too. If checkpatch really complains about it,
> this is a bug in checkpatch.
I think you are right. "else {return IRQ_NONE;}" gives no check patch warning.
>
> Regards,
>
>   Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAksgy7kACgkQD27XaX1/VRuNlwCgsSRFNBn9ZOhqnQrNuO8T+jGD
> 0IcAnj31td4yugsGcccmKyPPkNTogzGt
> =u741
> -----END PGP SIGNATURE-----
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html