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

[Dan Carpenter]

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@xxxxxxxxx>
                 ^^^^^^^^
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