linux-kernel - Re: [PATCH] Staging : rtl8188eu : rtw_security.c - Fixed warning: coding style issues - Fixed warning: if statement containing return with an else

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190805113145.GB1974@kadam>
Date:   Mon, 5 Aug 2019 14:31:45 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     merwintf <merwintf@...il.com>
Cc:     Larry.Finger@...inger.net, gregkh@...uxfoundation.org,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging : rtl8188eu : rtw_security.c       - Fixed
 warning: coding style issues   - Fixed warning: if statement containing
 return with an else    - Fixed check: coding style issues


1) Fix the From header.
2) Fix the subject.
3) Add a blank line after the subject.
4) Split the path up into multiple patches that each do one kind of
   change.

On Mon, Aug 05, 2019 at 01:22:56PM +0530, merwintf wrote:
> Signed-off-by: merwintf <merwintf@...il.com>
                 ^^^^^^^^
Use your real name like for a legal document.

>  static u8 crc32_reverseBit(u8 data)
>  {
> -	return (u8)((data<<7)&0x80) | ((data<<5)&0x40) | ((data<<3)&0x20) |
> -		   ((data<<1)&0x10) | ((data>>1)&0x08) | ((data>>3)&0x04) |
> -		   ((data>>5)&0x02) | ((data>>7)&0x01);
> +	return (u8)((data << 7) & 0x80)
> +		 | ((data << 5) & 0x40)
> +		 | ((data << 3) & 0x20)
> +		 | ((data << 1) & 0x10)
> +		 | ((data >> 1) & 0x08)
> +		 | ((data >> 3) & 0x04)
> +		 | ((data >> 5) & 0x02)
> +		 | ((data >> 7) & 0x01);


Put the | at the end of the line, not the start.  The cast isn't
required and it kind of messes up the white space so just leave it out
so that we don't have to change this twice.

> +	return (u8)((data << 7) & 0x80)
> +		 | ((data << 5) & 0x40)
> +		 | ((data << 3) & 0x20)
> +		 | ((data << 1) & 0x10)
> +		 | ((data >> 1) & 0x08)
> +		 | ((data >> 3) & 0x04)
> +		 | ((data >> 5) & 0x02)
> +		 | ((data >> 7) & 0x01);

	return ((data << 7) & 0x80) |
	       ((data << 5) & 0x40) |

etc.


>  }
>  
>  static void crc32_init(void)
>  {
> -	if (bcrc32initialized == 1) {
> -		return;
> -	} else {
> +	if (bcrc32initialized != 1) {

This isn't really an improvement.  Move the declarations outside the
block and do it like this:

	int i, j;
	u32 c;
	u8 *p = (u8 *)&c, *p1;

	if (bcrc32initialized == 1)
		return;

> @@ -164,7 +172,8 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>  		return;
>  
>  	if (crypto_ops->set_key(psecuritypriv->dot11DefKey[keyindex].skey,
> -				psecuritypriv->dot11DefKeylen[keyindex], NULL, crypto_private) < 0)
> +				psecuritypriv->dot11DefKeylen[keyindex],
> +				NULL, crypto_private) < 0)
>  		goto free_crypto_private;

Introduce an "int ret;" or something.

	ret = crypto_ops->set_key();
	if (ret < 0)
		goto free_crypto_private;




> @@ -201,16 +211,20 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>  
>  int rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  {
> -	struct	rx_pkt_attrib	 *prxattrib = &(((struct recv_frame *)precvframe)->attrib);
> +	struct	rx_pkt_attrib	 *prxattrib =
> +				  &(((struct recv_frame *)precvframe)->attrib);

This change isn't an improvement.

Anyway, hopefully that gives you some ideas.  But split up the patch.

regards,
dan carpenter