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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
