[PATCH 00/13] staging:rtl8192u: Style & memory leak fix

[PATCH 00/13] staging:rtl8192u: Style & memory leak fix

[John Whitmore]

Back again. I previously went through the struct ht_capability_ele
renaming member variables as being unused. At the time I wasn't
100% sure that the structure wasn't memory mapped, or used by
external callers to the module, through function pointers or exported
symbols. Having had a more detailed look over the code I'm happy
that these member variables are not used and have removed them from
the code.

The last patch is a potential memory leak, which only happens if two
memory allocations succeed and the third allocation fails. So it's
probably never going to happen, but fixed it anyhow.

I've tried to find this device, or a datasheet on it, so that I
could actually make sure it still worked but that's not been possible.
Apparently it's a discontinued device, but that possibly makes it a
safer sandbox to start messing with.

John Whitmore (13):
  staging:rtl8192u: Remove HTHalfMcsToDataRate() - Style
  staging:rtl8192u: Make HTMcsToDataRate static - Style
  staging:rtl8192u: Remove definition of HTSetConnectBwMode - Style
  staging:rtl8192u: Move HTSetConnectBwModeCallback() - Style
  staging:rtl8192u: Remove AdvCoding and GreenField - Style
  staging:rtl8192u: Remove TxSTBC and RxSTBC - Style
  staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style
  staging:rtl8192u: Remove LSigTxopProtect - Style
  staging:rtl8192u: Remove Rsvd2 - Style
  staging:rtl8192u: Remove ExtHTCapInfo - Style
  staging:rtl8192u: Remove TxBFCap - Style
  staging:rtl8192u: Remove ASCap - Style
  staging:rtl8192u: Remove potential memory leak

 .../staging/rtl8192u/ieee80211/ieee80211.h    |   3 -
 .../rtl8192u/ieee80211/ieee80211_module.c     |   5 +
 .../staging/rtl8192u/ieee80211/rtl819x_HT.h   |  20 +--
 .../rtl8192u/ieee80211/rtl819x_HTProc.c       | 123 ++++--------------
 4 files changed, 30 insertions(+), 121 deletions(-)

-- 
2.18.0

[PATCH 01/13] staging:rtl8192u: Remove HTHalfMcsToDataRate() - Style

[John Whitmore]

The function HTHalfMcsToDataRate() is unused in code so has simply
been removed from the code.

As a result two static functions, (IsHTHalfNmode40Bandwidth and
IsHTHalfNmodeSGI), are not longer called, so they have been removed
as well.

These are coding style changes which should have no impact on
runtime code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 .../staging/rtl8192u/ieee80211/ieee80211.h    |  1 -
 .../rtl8192u/ieee80211/rtl819x_HTProc.c       | 57 -------------------
 2 files changed, 58 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index b4e7ae60ac1c..cc694e567040 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2382,7 +2382,6 @@ u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame);
 //extern void HTSetConnectBwModeCallback(unsigned long data);
 void HTResetIOTSetting(PRT_HIGH_THROUGHPUT pHTInfo);
 bool IsHTHalfNmodeAPs(struct ieee80211_device *ieee);
-u16 HTHalfMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate);
 u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate);
 u16 TxCountToDataRate(struct ieee80211_device *ieee, u8 nDataRate);
 //function in BAPROC.c
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 9bf52cebe4cd..ea1261e33a68 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -216,63 +216,6 @@ void HTDebugHTInfo(u8 *InfoIE, u8 *TitleString)
 				pHTInfoEle->BasicMSC[1], pHTInfoEle->BasicMSC[2], pHTInfoEle->BasicMSC[3], pHTInfoEle->BasicMSC[4]);
 }
 
-/*
- *	Return:		true if station in half n mode and AP supports 40 bw
- */
-static bool IsHTHalfNmode40Bandwidth(struct ieee80211_device *ieee)
-{
-	bool			retValue = false;
-	PRT_HIGH_THROUGHPUT	 pHTInfo = ieee->pHTInfo;
-
-	if (!pHTInfo->bCurrentHTSupport)		// wireless is n mode
-		retValue = false;
-	else if (!pHTInfo->bRegBW40MHz)		// station supports 40 bw
-		retValue = false;
-	else if (!ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev))	// station in half n mode
-		retValue = false;
-	else if (((struct ht_capability_ele *)(pHTInfo->PeerHTCapBuf))->ChlWidth) // ap support 40 bw
-		retValue = true;
-	else
-		retValue = false;
-
-	return retValue;
-}
-
-static bool IsHTHalfNmodeSGI(struct ieee80211_device *ieee, bool is40MHz)
-{
-	bool			retValue = false;
-	PRT_HIGH_THROUGHPUT	 pHTInfo = ieee->pHTInfo;
-
-	if (!pHTInfo->bCurrentHTSupport)		// wireless is n mode
-		retValue = false;
-	else if (!ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev))	// station in half n mode
-		retValue = false;
-	else if (is40MHz) { // ap support 40 bw
-		if (((struct ht_capability_ele *)(pHTInfo->PeerHTCapBuf))->ShortGI40Mhz) // ap support 40 bw short GI
-			retValue = true;
-		else
-			retValue = false;
-	} else {
-		if (((struct ht_capability_ele *)(pHTInfo->PeerHTCapBuf))->ShortGI20Mhz) // ap support 40 bw short GI
-			retValue = true;
-		else
-			retValue = false;
-	}
-
-	return retValue;
-}
-
-u16 HTHalfMcsToDataRate(struct ieee80211_device *ieee,	u8	nMcsRate)
-{
-	u8	is40MHz;
-	u8	isShortGI;
-
-	is40MHz = (IsHTHalfNmode40Bandwidth(ieee)) ? 1 : 0;
-	isShortGI = (IsHTHalfNmodeSGI(ieee, is40MHz)) ? 1 : 0;
-
-	return MCS_DATA_RATE[is40MHz][isShortGI][(nMcsRate & 0x7f)];
-}
-
 u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
 {
 	PRT_HIGH_THROUGHPUT	pHTInfo = ieee->pHTInfo;
-- 
2.18.0

[PATCH 02/13] staging:rtl8192u: Make HTMcsToDataRate static - Style

[John Whitmore]

The function HTMcsToDataRate() is not used outside the file in which
it is defined, so has been declared as 'static' and the prototype
removed from the header file.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211.h      | 1 -
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index cc694e567040..8c91cb29fc9d 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2382,7 +2382,6 @@ u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame);
 //extern void HTSetConnectBwModeCallback(unsigned long data);
 void HTResetIOTSetting(PRT_HIGH_THROUGHPUT pHTInfo);
 bool IsHTHalfNmodeAPs(struct ieee80211_device *ieee);
-u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate);
 u16 TxCountToDataRate(struct ieee80211_device *ieee, u8 nDataRate);
 //function in BAPROC.c
 int ieee80211_rx_ADDBAReq(struct ieee80211_device *ieee, struct sk_buff *skb);
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index ea1261e33a68..ebe8d527a080 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -216,7 +216,7 @@ void HTDebugHTInfo(u8 *InfoIE, u8 *TitleString)
 				pHTInfoEle->BasicMSC[1], pHTInfoEle->BasicMSC[2], pHTInfoEle->BasicMSC[3], pHTInfoEle->BasicMSC[4]);
 }
 
-u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
+static u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
 {
 	PRT_HIGH_THROUGHPUT	pHTInfo = ieee->pHTInfo;
 
-- 
2.18.0

[PATCH 03/13] staging:rtl8192u: Remove definition of HTSetConnectBwMode - Style

[John Whitmore]

Remove the redundant declaration of the function HTSetConnectBwMode()
as it is not needed in the code. The function is already declared
in header file, multiple declarations add nothing.

This is a style change which should have no impact on runtime code
execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index ebe8d527a080..977a1e5c9b52 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -837,7 +837,6 @@ static u8 HTFilterMCSRate(struct ieee80211_device *ieee, u8 *pSupportMCS,
 	return true;
 }
 
-void HTSetConnectBwMode(struct ieee80211_device *ieee, enum ht_channel_width Bandwidth, enum ht_extension_chan_offset	Offset);
 void HTOnAssocRsp(struct ieee80211_device *ieee)
 {
 	PRT_HIGH_THROUGHPUT	pHTInfo = ieee->pHTInfo;
-- 
2.18.0

[PATCH 04/13] staging:rtl8192u: Move HTSetConnectBwModeCallback() - Style

[John Whitmore]

The function HTSetConnectBwModeCallback() is only used in the file
in which it is defined, so has been changed to being of type 'static'.

Additionally the function is defined after it is used so a prototype
was included in the file. This prototype, in the middle of the file
has been removed and the function implementation moved so that it is
defined before it is used in the file.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 .../staging/rtl8192u/ieee80211/ieee80211.h    |  1 -
 .../rtl8192u/ieee80211/rtl819x_HTProc.c       | 47 +++++++++----------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 8c91cb29fc9d..8aa536d79900 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2379,7 +2379,6 @@ u8 HTGetHighestMCSRate(struct ieee80211_device *ieee,
 extern u8 MCS_FILTER_ALL[];
 extern u16 MCS_DATA_RATE[2][2][77];
 u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame);
-//extern void HTSetConnectBwModeCallback(unsigned long data);
 void HTResetIOTSetting(PRT_HIGH_THROUGHPUT pHTInfo);
 bool IsHTHalfNmodeAPs(struct ieee80211_device *ieee);
 u16 TxCountToDataRate(struct ieee80211_device *ieee, u8 nDataRate);
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 977a1e5c9b52..c73a8058cf87 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -1011,7 +1011,6 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
 	pHTInfo->CurrentOpMode = pPeerHTInfo->OptMode;
 }
 
-void HTSetConnectBwModeCallback(struct ieee80211_device *ieee);
 /*
  *function:  initialize HT info(struct PRT_HIGH_THROUGHPUT)
  *   input:  struct ieee80211_device*	ieee
@@ -1228,6 +1227,29 @@ u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame)
 	return false;
 }
 
+static void HTSetConnectBwModeCallback(struct ieee80211_device *ieee)
+{
+	PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
+
+	IEEE80211_DEBUG(IEEE80211_DL_HT, "======>%s()\n", __func__);
+
+	if (pHTInfo->bCurBW40MHz) {
+		if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_UPPER)
+			ieee->set_chan(ieee->dev, ieee->current_network.channel + 2);
+		else if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_LOWER)
+			ieee->set_chan(ieee->dev, ieee->current_network.channel - 2);
+		else
+			ieee->set_chan(ieee->dev, ieee->current_network.channel);
+
+		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20_40, pHTInfo->CurSTAExtChnlOffset);
+	} else {
+		ieee->set_chan(ieee->dev, ieee->current_network.channel);
+		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
+	}
+
+	pHTInfo->bSwBwInProgress = false;
+}
+
 /*
  * This function set bandwidth mode in protocol layer.
  */
@@ -1278,26 +1300,3 @@ void HTSetConnectBwMode(struct ieee80211_device *ieee, enum ht_channel_width Ban
 
 //	spin_unlock_irqrestore(&(ieee->bw_spinlock), flags);
 }
-
-void HTSetConnectBwModeCallback(struct ieee80211_device *ieee)
-{
-	PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
-
-	IEEE80211_DEBUG(IEEE80211_DL_HT, "======>%s()\n", __func__);
-
-	if (pHTInfo->bCurBW40MHz) {
-		if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_UPPER)
-			ieee->set_chan(ieee->dev, ieee->current_network.channel + 2);
-		else if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_LOWER)
-			ieee->set_chan(ieee->dev, ieee->current_network.channel - 2);
-		else
-			ieee->set_chan(ieee->dev, ieee->current_network.channel);
-
-		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20_40, pHTInfo->CurSTAExtChnlOffset);
-	} else {
-		ieee->set_chan(ieee->dev, ieee->current_network.channel);
-		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
-	}
-
-	pHTInfo->bSwBwInProgress = false;
-}
-- 
2.18.0

[PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

[John Whitmore]

The member variables AdvCoding and GreenField are unused in code so
have been removed from the structure and associated initialisation
function.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 64d5359cf7e2..83fb8f34ccbd 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
 
 struct ht_capability_ele {
 	//HT capability info
-	u8	AdvCoding:1;
 	u8	ChlWidth:1;
 	u8	MimoPwrSave:2;
-	u8	GreenField:1;
 	u8	ShortGI20Mhz:1;
 	u8	ShortGI40Mhz:1;
 	u8	TxSTBC:1;
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index c73a8058cf87..d1fbe65ed428 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -493,7 +493,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 	}
 
 	//HT capability info
-	pCapELE->AdvCoding		= 0; // This feature is not supported now!!
 	if (ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev))
 		pCapELE->ChlWidth = 0;
 	else
@@ -501,7 +500,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 
 //	pCapELE->ChlWidth		= (pHT->bRegBW40MHz?1:0);
 	pCapELE->MimoPwrSave		= pHT->SelfMimoPs;
-	pCapELE->GreenField		= 0; // This feature is not supported now!!
 	pCapELE->ShortGI20Mhz		= 1; // We can receive Short GI!!
 	pCapELE->ShortGI40Mhz		= 1; // We can receive Short GI!!
 	//DbgPrint("TX HT cap/info ele BW=%d SG20=%d SG40=%d\n\r",
-- 
2.18.0

Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

[Greg KH]

On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
> 
> This is a coding style change which should have no impact on runtime
> code execution.
> 
> Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>  
>  struct ht_capability_ele {
>  	//HT capability info
> -	u8	AdvCoding:1;
>  	u8	ChlWidth:1;
>  	u8	MimoPwrSave:2;
> -	u8	GreenField:1;

Don't these fields come from the hardware itself?  By removing them
here, you just changed the memory layout of the structure.  Does the
driver still work properly after this?  If you can't test it, I can't
take this patch as it's too risky...

sorry,

greg k-h

Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

[John Whitmore]

On Fri, Sep 28, 2018 at 02:35:50PM +0200, Greg KH wrote:
> On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> > The member variables AdvCoding and GreenField are unused in code so
> > have been removed from the structure and associated initialisation
> > function.
> > 
> > This is a coding style change which should have no impact on runtime
> > code execution.
> > 
> > Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> >  2 files changed, 4 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > index 64d5359cf7e2..83fb8f34ccbd 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
> >  
> >  struct ht_capability_ele {
> >  	//HT capability info
> > -	u8	AdvCoding:1;
> >  	u8	ChlWidth:1;
> >  	u8	MimoPwrSave:2;
> > -	u8	GreenField:1;
> 
> Don't these fields come from the hardware itself?  By removing them
> here, you just changed the memory layout of the structure.  Does the
> driver still work properly after this?  If you can't test it, I can't
> take this patch as it's too risky...
> 

Sorry, yes the structure looks like it should come from the hardware
but as the structure is allocated from memory I expected to find a
memcopy either to or from the hardware. Yes risky, just because I
couldn't find it don't mean the connection to hardware ain't there.

I'll lay off the risky and who knows if I keep wondering through the
driver I'll find that illusive connection.

Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

[Dan Carpenter]

On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
> 
> This is a coding style change which should have no impact on runtime
> code execution.
> 
> Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>  
>  struct ht_capability_ele {
>  	//HT capability info
> -	u8	AdvCoding:1;
>  	u8	ChlWidth:1;
>  	u8	MimoPwrSave:2;
> -	u8	GreenField:1;
>  	u8	ShortGI20Mhz:1;
>  	u8	ShortGI40Mhz:1;
>  	u8	TxSTBC:1;

I feel like we discussed this before.  I'm pretty sure this comes from
the firmware and so the format can't be changed.  When I look at
rtllib_parse_mife_generic() then I think that "info_element" probably
comes from the firmware.

I wouldn't want to accept this with out someone testing it.

regards,
dan carpenter

Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

[John Whitmore]

On Fri, Sep 28, 2018 at 05:31:40PM +0300, Dan Carpenter wrote:
> On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> > The member variables AdvCoding and GreenField are unused in code so
> > have been removed from the structure and associated initialisation
> > function.
> > 
> > This is a coding style change which should have no impact on runtime
> > code execution.
> > 
> > Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> >  2 files changed, 4 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > index 64d5359cf7e2..83fb8f34ccbd 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
> >  
> >  struct ht_capability_ele {
> >  	//HT capability info
> > -	u8	AdvCoding:1;
> >  	u8	ChlWidth:1;
> >  	u8	MimoPwrSave:2;
> > -	u8	GreenField:1;
> >  	u8	ShortGI20Mhz:1;
> >  	u8	ShortGI40Mhz:1;
> >  	u8	TxSTBC:1;
> 
> I feel like we discussed this before.  I'm pretty sure this comes from
> the firmware and so the format can't be changed.  When I look at
> rtllib_parse_mife_generic() then I think that "info_element" probably
> comes from the firmware.
> 
> I wouldn't want to accept this with out someone testing it.
> 
> regards,
> dan carpenter
> 

Thank you and sorry about not helping the signal to noise ratio on here.

I agree that a bit field like that and it looks like it comes from
firmware, but my question or possibly obsession was where. There are
structures inside structures, but they are all allocated from RAM.
Because of that I expected to find a memcopy from the device, or given
that the bitfield is initialised with values that it might be memcopy'd
to the device. I just couldn't find that memcopy, but that's down to
my untrained eye. I'll stumble across it in some obscure corner of the
driver.

[PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

[John Whitmore]

Remove the member variables TxSTBC and RxSTBC as neither is used in
code.

This is a coding style change which should not impact runtime code
execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 83fb8f34ccbd..52cce0dcf9a5 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -43,8 +43,6 @@ struct ht_capability_ele {
 	u8	MimoPwrSave:2;
 	u8	ShortGI20Mhz:1;
 	u8	ShortGI40Mhz:1;
-	u8	TxSTBC:1;
-	u8	RxSTBC:2;
 	u8	DelayBA:1;
 	u8	MaxAMSDUSize:1;
 	u8	DssCCk:1;
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index d1fbe65ed428..b4220df1f833 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -145,7 +145,6 @@ void HTDebugHTCapability(u8 *CapIE, u8 *TitleString)
 	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tSupported Channel Width = %s\n", (pCapELE->ChlWidth) ? "20MHz" : "20/40MHz");
 	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tSupport Short GI for 20M = %s\n", (pCapELE->ShortGI20Mhz) ? "YES" : "NO");
 	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tSupport Short GI for 40M = %s\n", (pCapELE->ShortGI40Mhz) ? "YES" : "NO");
-	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tSupport TX STBC = %s\n", (pCapELE->TxSTBC) ? "YES" : "NO");
 	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tMax AMSDU Size = %s\n", (pCapELE->MaxAMSDUSize) ? "3839" : "7935");
 	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tSupport CCK in 20/40 mode = %s\n", (pCapELE->DssCCk) ? "YES" : "NO");
 	IEEE80211_DEBUG(IEEE80211_DL_HT,  "\tMax AMPDU Factor = %d\n", pCapELE->MaxRxAMPDUFactor);
@@ -504,8 +503,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 	pCapELE->ShortGI40Mhz		= 1; // We can receive Short GI!!
 	//DbgPrint("TX HT cap/info ele BW=%d SG20=%d SG40=%d\n\r",
 	//pCapELE->ChlWidth, pCapELE->ShortGI20Mhz, pCapELE->ShortGI40Mhz);
-	pCapELE->TxSTBC			= 1;
-	pCapELE->RxSTBC			= 0;
 	pCapELE->DelayBA		= 0;	// Do not support now!!
 	pCapELE->MaxAMSDUSize	        = (MAX_RECEIVE_BUFFER_SIZE >= 7935) ? 1 : 0;
 	pCapELE->DssCCk			= ((pHT->bRegBW40MHz) ? (pHT->bRegSuppCCK ? 1 : 0) : 0);
-- 
2.18.0

Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

[Greg KH]

On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote:
> Remove the member variables TxSTBC and RxSTBC as neither is used in
> code.
> 
> This is a coding style change which should not impact runtime code
> execution.

Same as before, I think this _does_ impact runtime :(

I'll stop here in the series.

thanks,

greg k-h

Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

[Dan Carpenter]

On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote:
> Remove the member variables TxSTBC and RxSTBC as neither is used in
> code.
> 
> This is a coding style change which should not impact runtime code
> execution.
> 
> Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 --
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
>  2 files changed, 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 83fb8f34ccbd..52cce0dcf9a5 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -43,8 +43,6 @@ struct ht_capability_ele {
>  	u8	MimoPwrSave:2;
>  	u8	ShortGI20Mhz:1;
>  	u8	ShortGI40Mhz:1;
> -	u8	TxSTBC:1;
> -	u8	RxSTBC:2;
>  	u8	DelayBA:1;
>  	u8	MaxAMSDUSize:1;
>  	u8	DssCCk:1;

Same thing.  I suspect his is set by the firmware.

regards,
dan carpenter

[PATCH 07/13] staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style

[John Whitmore]

Remove the unused member variables DelayBA, PSMP and Rsvd1.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 3 ---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
 2 files changed, 5 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 52cce0dcf9a5..09596a7a6814 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -43,11 +43,8 @@ struct ht_capability_ele {
 	u8	MimoPwrSave:2;
 	u8	ShortGI20Mhz:1;
 	u8	ShortGI40Mhz:1;
-	u8	DelayBA:1;
 	u8	MaxAMSDUSize:1;
 	u8	DssCCk:1;
-	u8	PSMP:1;
-	u8	Rsvd1:1;
 	u8	LSigTxopProtect:1;
 
 	//MAC HT parameters info
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index b4220df1f833..94cedf5f1163 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -503,10 +503,8 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 	pCapELE->ShortGI40Mhz		= 1; // We can receive Short GI!!
 	//DbgPrint("TX HT cap/info ele BW=%d SG20=%d SG40=%d\n\r",
 	//pCapELE->ChlWidth, pCapELE->ShortGI20Mhz, pCapELE->ShortGI40Mhz);
-	pCapELE->DelayBA		= 0;	// Do not support now!!
 	pCapELE->MaxAMSDUSize	        = (MAX_RECEIVE_BUFFER_SIZE >= 7935) ? 1 : 0;
 	pCapELE->DssCCk			= ((pHT->bRegBW40MHz) ? (pHT->bRegSuppCCK ? 1 : 0) : 0);
-	pCapELE->PSMP			= 0; // Do not support now!!
 	pCapELE->LSigTxopProtect	= 0; // Do not support now!!
 
 	/*
-- 
2.18.0

Re: [PATCH 07/13] staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style

[Dan Carpenter]

Yeah...  :(  All the remaining patches are similar and risky.

regards,
dan carpenter

[PATCH 08/13] staging:rtl8192u: Remove LSigTxopProtect - Style

[John Whitmore]

Remove the unused member bit LSigTxopProtect. This member is not used
in code so has been replaced with a 'padding' bit, to keep byte
boundary alignment.

This is a coding style change which should not impact runtime code
execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 2 +-
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 09596a7a6814..4154b4e27e59 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -45,7 +45,7 @@ struct ht_capability_ele {
 	u8	ShortGI40Mhz:1;
 	u8	MaxAMSDUSize:1;
 	u8	DssCCk:1;
-	u8	LSigTxopProtect:1;
+	u8	padding:1;
 
 	//MAC HT parameters info
 	u8	MaxRxAMPDUFactor:2;
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 94cedf5f1163..58c94440aab0 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -505,7 +505,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 	//pCapELE->ChlWidth, pCapELE->ShortGI20Mhz, pCapELE->ShortGI40Mhz);
 	pCapELE->MaxAMSDUSize	        = (MAX_RECEIVE_BUFFER_SIZE >= 7935) ? 1 : 0;
 	pCapELE->DssCCk			= ((pHT->bRegBW40MHz) ? (pHT->bRegSuppCCK ? 1 : 0) : 0);
-	pCapELE->LSigTxopProtect	= 0; // Do not support now!!
 
 	/*
 	 * MAC HT parameters info
-- 
2.18.0

[PATCH 09/13] staging:rtl8192u: Remove Rsvd2 - Style

[John Whitmore]

Remove the unused member variable Rsvd2 from structure.

This is a coding style change which should not impact runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 4154b4e27e59..0c5ded7ad98c 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -50,7 +50,6 @@ struct ht_capability_ele {
 	//MAC HT parameters info
 	u8	MaxRxAMPDUFactor:2;
 	u8	MPDUDensity:3;
-	u8	Rsvd2:3;
 
 	//Supported MCS set
 	u8	MCS[16];
-- 
2.18.0

[PATCH 10/13] staging:rtl8192u: Remove ExtHTCapInfo - Style

[John Whitmore]

The member variable ExtHTCapInfo is not used in code so has been
removed.

This is a coding style change which should not impact runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 3 ---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 0c5ded7ad98c..f36fbf56c585 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -54,9 +54,6 @@ struct ht_capability_ele {
 	//Supported MCS set
 	u8	MCS[16];
 
-	//Extended HT Capability Info
-	u16	ExtHTCapInfo;
-
 	//TXBF Capabilities
 	u8	TxBFCap[4];
 
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 58c94440aab0..e20488ecbd96 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -542,9 +542,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 			pCapELE->MCS[i] = 0;
 	}
 
-	//Extended HT Capability Info
-	memset(&pCapELE->ExtHTCapInfo, 0, 2);
-
 	//TXBF Capabilities
 	memset(pCapELE->TxBFCap, 0, 4);
 
-- 
2.18.0

[PATCH 11/13] staging:rtl8192u: Remove TxBFCap - Style

[John Whitmore]

The member variable TxBFCap is not used in code so has simply been
removed from the code.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 3 ---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index f36fbf56c585..204e4a5495a1 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -54,9 +54,6 @@ struct ht_capability_ele {
 	//Supported MCS set
 	u8	MCS[16];
 
-	//TXBF Capabilities
-	u8	TxBFCap[4];
-
 	//Antenna Selection Capabilities
 	u8	ASCap;
 
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index e20488ecbd96..9188bd925262 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -542,9 +542,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 			pCapELE->MCS[i] = 0;
 	}
 
-	//TXBF Capabilities
-	memset(pCapELE->TxBFCap, 0, 4);
-
 	//Antenna Selection Capabilities
 	pCapELE->ASCap = 0;
 //add 2 to give space for element ID and len when construct frames
-- 
2.18.0

[PATCH 12/13] staging:rtl8192u: Remove ASCap - Style

[John Whitmore]

Remove the unused member variable ASCap from code.

This is a coding style change which should not impact runtime code
execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h     | 4 ----
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 204e4a5495a1..85a8716c6b05 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -53,10 +53,6 @@ struct ht_capability_ele {
 
 	//Supported MCS set
 	u8	MCS[16];
-
-	//Antenna Selection Capabilities
-	u8	ASCap;
-
 } __packed;
 
 /*
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 9188bd925262..4f6e9da465e2 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -542,8 +542,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 			pCapELE->MCS[i] = 0;
 	}
 
-	//Antenna Selection Capabilities
-	pCapELE->ASCap = 0;
 //add 2 to give space for element ID and len when construct frames
 	if (pHT->ePeerHTSpecVer == HT_SPEC_VER_EWC)
 		*len = 30 + 2;
-- 
2.18.0

[PATCH 13/13] staging:rtl8192u: Remove potential memory leak

[John Whitmore]

Add call to ieee80211_networks_free() to avoid potential memory
leak if allocation of pHTInfo fails.

If the third allocation fails only the first successful allocation
is freed, not the second.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_module.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index 90a097f2cd4e..aada077ced0f 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -159,6 +159,11 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
 	ieee->pHTInfo = kzalloc(sizeof(RT_HIGH_THROUGHPUT), GFP_KERNEL);
 	if (ieee->pHTInfo == NULL) {
 		IEEE80211_DEBUG(IEEE80211_DL_ERR, "can't alloc memory for HTInfo\n");
+
+		/* By this point in code ieee80211_networks_allocate() has been
+		 * successfully called so the memory allocated should be freed
+		 */
+		ieee80211_networks_free(ieee);
 		goto failed;
 	}
 	HTUpdateDefaultSetting(ieee);
-- 
2.18.0