netdev - Re: [PATCH V2 2/4] net: can: ifi: Fix TX DLC configuration

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] [day] [month] [year] [list]

Message-ID: <56D74995.8010907@denx.de>
Date:	Wed, 02 Mar 2016 21:14:13 +0100
From:	Marek Vasut <marex@...x.de>
To:	Oliver Hartkopp <socketcan@...tkopp.net>, linux-can@...r.kernel.org
CC:	netdev@...r.kernel.org, Marc Kleine-Budde <mkl@...gutronix.de>,
	Mark Rutland <mark.rutland@....com>,
	Wolfgang Grandegger <wg@...ndegger.com>
Subject: Re: [PATCH V2 2/4] net: can: ifi: Fix TX DLC configuration

On 03/02/2016 07:46 PM, Oliver Hartkopp wrote:
> Hi Marek,

Hi!

> sorry for picking this patch up again.

nothing to be sorry about, thanks for the input :)

> After looking around in the original source I have one more comment:
> 
> On 03/02/2016 11:42 AM, Marek Vasut wrote:
> 
>> -	if (priv->can.ctrlmode & (CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO)) {
>> -		if (can_is_canfd_skb(skb)) {
>> -			txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
>> -			if (cf->flags & CANFD_BRS)
>> -				txdlc |= IFI_CANFD_TXFIFO_DLC_BRS;
>> -		}
>> +	txdlc |= can_len2dlc(cf->len);
> 
> At the top of the function you defined
> 
> 	u32 txst, txid;
> 	u32 txdlc = 0;
> 
> and here you use 'txdlc |= ...'.
> 
> That's weird style IMO.
> 
> Please change it to
> 
> 	u32 txst, txid, txdlc;
> 
> and
> 
> 	txdlc = can_len2dlc(cf->len);

I'll send a V3 shortly.

btw. I forgot to add V2 to the 3/4 and 4/4 patches, sigh.


-- 
Best regards,
Marek Vasut