[RFC 2/5] rtl8192u: fix braces in r8192U_core.c

[Dan Carpenter]

This one will need to be redone.  It introduces new GCC warnings:

drivers/staging/rtl8192u/r8192U_core.c:1958:9: warning: mixing declarations and code
drivers/staging/rtl8192u/r8192U_core.c:2698:25: warning: mixing declarations and code
drivers/staging/rtl8192u/r8192U_core.c:2809:9: warning: mixing declarations and code
drivers/staging/rtl8192u/r8192U_core.c:2895:9: warning: mixing declarations and code
drivers/staging/rtl8192u/r8192U_core.c:3007:17: warning: mixing declarations and code
drivers/staging/rtl8192u/r8192U_core.c:3019:9: warning: mixing declarations and code

People use this trick:
	{
		int my_local_var;

		blah; blah; blah;
	}

You will need to move the declaration to start of the function when
you delete the block.

Often the code does this:

#ifdef DEAD_CODE
	{
		int my_local_var;

		blah; blah; blah;
	}
#endif

My suggestion is to go through and delete the dead code first in a
separate patch.  Also really this patch is pretty huge.  With all
the indent changes and everything it's a bit hard to review, and I
knew you were going to have to redo it anyway so I didn't review
until the end.

One way to split this up would be:
[patch 1/3] remove ifdefed out dead code
[patch 2/3] move braces around but don't add or delete any brace or change indent levels
[patch 3/3] remove unneeded block statements and pull things in an indent level

The subjects are bad but you get the idea.

On Fri, May 31, 2013 at 08:10:49PM +0300, Xenia Ragiadakou wrote:
>  	if (priv->ieee80211->iw_mode == IW_MODE_MONITOR || \

You didn't add this, but these '\' characters are pointless.  This
isn't a macro.

> -	   dev->flags & IFF_PROMISC){
> +	   dev->flags & IFF_PROMISC)
>  		rxconf = rxconf | RCR_AAP;
> -	} else{
> +	else {

You can fix this in a follow on patch, but this isn't right.  The
rule is that if either side of the if else statement has curly
braces then both sides get them.

>  		rxconf = rxconf | RCR_APM;
>  		rxconf = rxconf | RCR_CBSSID;
>  	}
>  
>  
> @@ -1441,13 +1429,10 @@ void rtl8192_update_cap(struct net_device *dev, u16 cap)
>  		tmp |= BRSR_AckShortPmb;
>  	write_nic_dword(dev, RRSR, tmp);
>  
> -	if (net->mode & (IEEE_G|IEEE_N_24G))
> -	{
> +	if (net->mode & (IEEE_G|IEEE_N_24G)) {
>  		u8 slot_time = 0;
> -		if ((cap & WLAN_CAPABILITY_SHORT_SLOT)&&(!priv->ieee80211->pHTInfo->bCurrentRT2RTLongSlotTime))
> -		{//short slot time
> +		if ((cap & WLAN_CAPABILITY_SHORT_SLOT)&&(!priv->ieee80211->pHTInfo->bCurrentRT2RTLongSlotTime)) //short slot time
                                                                                                                  ^^^^^^^^^^^^^^^
This is a pointless comment.  Delete.

>  			slot_time = SHORT_SLOT_TIME;
> -		}
>  		else //long slot time
>  			slot_time = NON_SHORT_SLOT_TIME;
>  		priv->slot_time = slot_time;
> @@ -2563,15 +2503,12 @@ static void rtl8192_init_priv_variable(struct net_device *dev)
>  	skb_queue_head_init(&priv->skb_queue);
>  
>  	/* Tx related queue */
> -	for (i = 0; i < MAX_QUEUE_SIZE; i++) {
> +	for (i = 0; i < MAX_QUEUE_SIZE; i++)
>  		skb_queue_head_init(&priv->ieee80211->skb_waitQ [i]);
> -	}
> -	for (i = 0; i < MAX_QUEUE_SIZE; i++) {
> +	for (i = 0; i < MAX_QUEUE_SIZE; i++)
>  		skb_queue_head_init(&priv->ieee80211->skb_aggQ [i]);
> -	}
> -	for (i = 0; i < MAX_QUEUE_SIZE; i++) {
> +	for (i = 0; i < MAX_QUEUE_SIZE; i++)
>  		skb_queue_head_init(&priv->ieee80211->skb_drv_aggQ [i]);
> -	}

In a later patch you can change this to:

	for (i = 0; i < MAX_QUEUE_SIZE; i++) {
		skb_queue_head_init(&priv->ieee80211->skb_waitQ[i]);
 		skb_queue_head_init(&priv->ieee80211->skb_aggQ[i]);
  		skb_queue_head_init(&priv->ieee80211->skb_drv_aggQ[i]);
	}

And the 'Q' on the end is bad CamelCase and it's bad kernel style.
"skb_waitQ" is a horrible name.  Neither "skb" nor
"waitQ/wait_queue" actually tell you any information at all about
what it's for.  :/

regards,
dan carpenter