diff mbox series

[09/12] Staging: rtl8192e: Rename variable pBA

Message ID ZK84sYjc9uHIWZcr@basil
State New
Headers
Series [01/12] Staging: rtl8192e: Rename function ActivateBAEntry |

Commit Message

Tree Davies July 12, 2023, 11:35 p.m. UTC 
  Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by: Tree Davies <tdavies@darkphysics.net>
---
 drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
 drivers/staging/rtl8192e/rtllib.h         |   2 +-
 2 files changed, 54 insertions(+), 54 deletions(-)

Comments

Philipp Hortmann July 13, 2023, 9:54 p.m. UTC | #1 
On 7/13/23 01:35, Tree Davies wrote:
> Rename variable pBA to pba in order to Fix checkpatch
> warning: Avoid CamelCase
> 
> Signed-off-by: Tree Davies<tdavies@darkphysics.net>
> ---
>   drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
>   drivers/staging/rtl8192e/rtllib.h         |   2 +-
>   2 files changed, 54 insertions(+), 54 deletions(-)


Hi Tree,
the p is typically for pointer. This is not wanted when you change the 
name. But ba is is in use....

Bye Philipp
Tree Davies July 14, 2023, 2:49 a.m. UTC | #2 
On Thu, Jul 13, 2023 at 11:54:40PM +0200, Philipp Hortmann wrote:
> On 7/13/23 01:35, Tree Davies wrote:
> > Rename variable pBA to pba in order to Fix checkpatch
> > warning: Avoid CamelCase
> > 
> > Signed-off-by: Tree Davies<tdavies@darkphysics.net>
> > ---
> >   drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
> >   drivers/staging/rtl8192e/rtllib.h         |   2 +-
> >   2 files changed, 54 insertions(+), 54 deletions(-)
> 
> 
> Hi Tree,
> the p is typically for pointer. This is not wanted when you change the name.
> But ba is is in use....
> 
> Bye Philipp

Thanks Philipp,                                                      
                                                                     
A few thoughts...                                                    
Looking at occurances of pBA, they all appear as local variable      
declarations of struct ba_record, mostly as function params.         
                                                                     
I also see what you mentioned, as BA being already taken in          
rtl819x_BAProc.c:394 and line 292, but I don't 'think' that renaming them
both to ba will result negatively(?).

Agreed, let's wait on Greg.

Cheers!
Tree
Philipp Hortmann July 14, 2023, 4:57 a.m. UTC | #3 
On 7/14/23 04:49, Tree Davies wrote:
> On Thu, Jul 13, 2023 at 11:54:40PM +0200, Philipp Hortmann wrote:
>> On 7/13/23 01:35, Tree Davies wrote:
>>> Rename variable pBA to pba in order to Fix checkpatch
>>> warning: Avoid CamelCase
>>>
>>> Signed-off-by: Tree Davies<tdavies@darkphysics.net>
>>> ---
>>>    drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
>>>    drivers/staging/rtl8192e/rtllib.h         |   2 +-
>>>    2 files changed, 54 insertions(+), 54 deletions(-)
>>
>>
>> Hi Tree,
>> the p is typically for pointer. This is not wanted when you change the name.
>> But ba is is in use....
>>
>> Bye Philipp
> 
> Thanks Philipp,
>                                                                       
> A few thoughts...
> Looking at occurances of pBA, they all appear as local variable
> declarations of struct ba_record, mostly as function params.
>                                                                       
> I also see what you mentioned, as BA being already taken in
> rtl819x_BAProc.c:394 and line 292, but I don't 'think' that renaming them
> both to ba will result negatively(?).
> 
> Agreed, let's wait on Greg.
> 
> Cheers!
> Tree
> 

Hi Tree,

it is not so much about the compiler or if it works this way. When I 
read a program I often use Ctrl+Shift+F what shows me all variables in 
one folder (driver). I expect that I then always see one variable for 
one use and do not have to do a one by one decision if this belongs to 
the one content or to the other.

Please consider "readability" is important for kernel code.

Thanks for your support.

Bye Philipp
Dan Carpenter July 14, 2023, 9:53 a.m. UTC | #4 
On Wed, Jul 12, 2023 at 04:35:13PM -0700, Tree Davies wrote:
> Rename variable pBA to pba in order to Fix checkpatch
> warning: Avoid CamelCase
> 
> Signed-off-by: Tree Davies <tdavies@darkphysics.net>
> ---
>  drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
>  drivers/staging/rtl8192e/rtllib.h         |   2 +-
>  2 files changed, 54 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> index 6b5da38353ee..43ee1bd4a6ed 100644
> --- a/drivers/staging/rtl8192e/rtl819x_BAProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> @@ -10,17 +10,17 @@
>  #include "rtllib.h"
>  #include "rtl819x_BA.h"
>  
> -static void activate_ba_entry(struct ba_record *pBA, u16 Time)
> +static void activate_ba_entry(struct ba_record *pba, u16 Time)

Here the "p" stands for "pointer".  This style of naming is not allowed
in the Linux kernel.  Just use "ba" as a name.  This happens in a couple
other patches as well so fix those too.

regards,
dan carpenter
Dan Carpenter July 14, 2023, 10:10 a.m. UTC | #5 
On Thu, Jul 13, 2023 at 07:49:39PM -0700, Tree Davies wrote:
> On Thu, Jul 13, 2023 at 11:54:40PM +0200, Philipp Hortmann wrote:
> > On 7/13/23 01:35, Tree Davies wrote:
> > > Rename variable pBA to pba in order to Fix checkpatch
> > > warning: Avoid CamelCase
> > > 
> > > Signed-off-by: Tree Davies<tdavies@darkphysics.net>
> > > ---
> > >   drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
> > >   drivers/staging/rtl8192e/rtllib.h         |   2 +-
> > >   2 files changed, 54 insertions(+), 54 deletions(-)
> > 
> > 
> > Hi Tree,
> > the p is typically for pointer. This is not wanted when you change the name.
> > But ba is is in use....
> > 
> > Bye Philipp
> 
> Thanks Philipp,                                                      
>                                                                      
> A few thoughts...                                                    
> Looking at occurances of pBA, they all appear as local variable      
> declarations of struct ba_record, mostly as function params.         

I'm reading my inbox in the wrong order so I already sent an email
with the same advice that Philipp sent.  Philipp is correct.  "p" is
not allowed.

>                                                                      
> I also see what you mentioned, as BA being already taken in          
> rtl819x_BAProc.c:394 and line 292, but I don't 'think' that renaming them
> both to ba will result negatively(?).

Thanks for noticing this.  Figure out a solution.

> 
> Agreed, let's wait on Greg.

Heh.  Greg is not going to fix these minor issues and neither am I.  We
trust you to find a solution.  You can do it!  I believe in you!

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c b/drivers/staging/rtl8192e/rtl819x_BAProc.c
index 6b5da38353ee..43ee1bd4a6ed 100644
--- a/drivers/staging/rtl8192e/rtl819x_BAProc.c
+++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c
@@ -10,17 +10,17 @@ 
 #include "rtllib.h"
 #include "rtl819x_BA.h"
 
-static void activate_ba_entry(struct ba_record *pBA, u16 Time)
+static void activate_ba_entry(struct ba_record *pba, u16 Time)
 {
-	pBA->b_valid = true;
+	pba->b_valid = true;
 	if (Time != 0)
-		mod_timer(&pBA->timer, jiffies + msecs_to_jiffies(Time));
+		mod_timer(&pba->timer, jiffies + msecs_to_jiffies(Time));
 }
 
-static void deactivate_ba_entry(struct rtllib_device *ieee, struct ba_record *pBA)
+static void deactivate_ba_entry(struct rtllib_device *ieee, struct ba_record *pba)
 {
-	pBA->b_valid = false;
-	del_timer_sync(&pBA->timer);
+	pba->b_valid = false;
+	del_timer_sync(&pba->timer);
 }
 
 static u8 tx_ts_delete_ba(struct rtllib_device *ieee, struct tx_ts_record *pTxTs)
@@ -43,28 +43,28 @@  static u8 tx_ts_delete_ba(struct rtllib_device *ieee, struct tx_ts_record *pTxTs
 
 static u8 RxTsDeleteBA(struct rtllib_device *ieee, struct rx_ts_record *pRxTs)
 {
-	struct ba_record *pBa = &pRxTs->rx_admitted_ba_record;
+	struct ba_record *pba = &pRxTs->rx_admitted_ba_record;
 	u8			bSendDELBA = false;
 
-	if (pBa->b_valid) {
-		deactivate_ba_entry(ieee, pBa);
+	if (pba->b_valid) {
+		deactivate_ba_entry(ieee, pba);
 		bSendDELBA = true;
 	}
 
 	return bSendDELBA;
 }
 
-void ResetBaEntry(struct ba_record *pBA)
+void ResetBaEntry(struct ba_record *pba)
 {
-	pBA->b_valid			  = false;
-	pBA->ba_param_set.short_data	  = 0;
-	pBA->ba_timeout_value		  = 0;
-	pBA->dialog_token		  = 0;
-	pBA->ba_start_seq_ctrl.short_data = 0;
+	pba->b_valid			  = false;
+	pba->ba_param_set.short_data	  = 0;
+	pba->ba_timeout_value		  = 0;
+	pba->dialog_token		  = 0;
+	pba->ba_start_seq_ctrl.short_data = 0;
 }
 
 static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
-				    struct ba_record *pBA,
+				    struct ba_record *pba,
 				    u16 StatusCode, u8 type)
 {
 	struct sk_buff *skb = NULL;
@@ -75,8 +75,8 @@  static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	netdev_dbg(ieee->dev, "%s(): frame(%d) sentd to: %pM, ieee->dev:%p\n",
 		   __func__, type, Dst, ieee->dev);
 
-	if (!pBA) {
-		netdev_warn(ieee->dev, "pBA is NULL\n");
+	if (!pba) {
+		netdev_warn(ieee->dev, "pba is NULL\n");
 		return NULL;
 	}
 	skb = dev_alloc_skb(len + sizeof(struct rtllib_hdr_3addr));
@@ -98,21 +98,21 @@  static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	tag = skb_put(skb, 9);
 	*tag++ = ACT_CAT_BA;
 	*tag++ = type;
-	*tag++ = pBA->dialog_token;
+	*tag++ = pba->dialog_token;
 
 	if (type == ACT_ADDBARSP) {
 		put_unaligned_le16(StatusCode, tag);
 		tag += 2;
 	}
 
-	put_unaligned_le16(pBA->ba_param_set.short_data, tag);
+	put_unaligned_le16(pba->ba_param_set.short_data, tag);
 	tag += 2;
 
-	put_unaligned_le16(pBA->ba_timeout_value, tag);
+	put_unaligned_le16(pba->ba_timeout_value, tag);
 	tag += 2;
 
 	if (type == ACT_ADDBAREQ) {
-		memcpy(tag, (u8 *)&pBA->ba_start_seq_ctrl, 2);
+		memcpy(tag, (u8 *)&pba->ba_start_seq_ctrl, 2);
 		tag += 2;
 	}
 
@@ -124,7 +124,7 @@  static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 }
 
 static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
-				    struct ba_record *pBA,
+				    struct ba_record *pba,
 				    enum tr_select TxRxSelect, u16 ReasonCode)
 {
 	union delba_param_set DelbaParamSet;
@@ -140,7 +140,7 @@  static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 	memset(&DelbaParamSet, 0, 2);
 
 	DelbaParamSet.field.initiator = (TxRxSelect == TX_DIR) ? 1 : 0;
-	DelbaParamSet.field.tid	= pBA->ba_param_set.field.tid;
+	DelbaParamSet.field.tid	= pba->ba_param_set.field.tid;
 
 	skb = dev_alloc_skb(len + sizeof(struct rtllib_hdr_3addr));
 	if (!skb)
@@ -174,11 +174,11 @@  static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 }
 
 static void rtllib_send_ADDBAReq(struct rtllib_device *ieee, u8 *dst,
-				 struct ba_record *pBA)
+				 struct ba_record *pba)
 {
 	struct sk_buff *skb;
 
-	skb = rtllib_ADDBA(ieee, dst, pBA, 0, ACT_ADDBAREQ);
+	skb = rtllib_ADDBA(ieee, dst, pba, 0, ACT_ADDBAREQ);
 
 	if (skb)
 		softmac_mgmt_xmit(skb, ieee);
@@ -187,11 +187,11 @@  static void rtllib_send_ADDBAReq(struct rtllib_device *ieee, u8 *dst,
 }
 
 static void rtllib_send_ADDBARsp(struct rtllib_device *ieee, u8 *dst,
-				 struct ba_record *pBA, u16 StatusCode)
+				 struct ba_record *pba, u16 StatusCode)
 {
 	struct sk_buff *skb;
 
-	skb = rtllib_ADDBA(ieee, dst, pBA, StatusCode, ACT_ADDBARSP);
+	skb = rtllib_ADDBA(ieee, dst, pba, StatusCode, ACT_ADDBARSP);
 	if (skb)
 		softmac_mgmt_xmit(skb, ieee);
 	else
@@ -199,12 +199,12 @@  static void rtllib_send_ADDBARsp(struct rtllib_device *ieee, u8 *dst,
 }
 
 static void rtllib_send_DELBA(struct rtllib_device *ieee, u8 *dst,
-			      struct ba_record *pBA, enum tr_select TxRxSelect,
+			      struct ba_record *pba, enum tr_select TxRxSelect,
 			      u16 ReasonCode)
 {
 	struct sk_buff *skb;
 
-	skb = rtllib_DELBA(ieee, dst, pBA, TxRxSelect, ReasonCode);
+	skb = rtllib_DELBA(ieee, dst, pba, TxRxSelect, ReasonCode);
 	if (skb)
 		softmac_mgmt_xmit(skb, ieee);
 	else
@@ -216,7 +216,7 @@  int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct sk_buff *skb)
 	struct rtllib_hdr_3addr *req = NULL;
 	u16 rc = 0;
 	u8 *dst = NULL, *pDialogToken = NULL, *tag = NULL;
-	struct ba_record *pBA = NULL;
+	struct ba_record *pba = NULL;
 	union ba_param_set *pBaParamSet = NULL;
 	u16 *pBaTimeoutVal = NULL;
 	union sequence_control *pBaStartSeqCtrl = NULL;
@@ -259,7 +259,7 @@  int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct sk_buff *skb)
 		netdev_warn(ieee->dev, "%s(): can't get TS\n", __func__);
 		goto OnADDBAReq_Fail;
 	}
-	pBA = &pTS->rx_admitted_ba_record;
+	pba = &pTS->rx_admitted_ba_record;
 
 	if (pBaParamSet->field.ba_policy == BA_POLICY_DELAYED) {
 		rc = ADDBA_STATUS_INVALID_PARAM;
@@ -270,20 +270,20 @@  int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct sk_buff *skb)
 
 	rtllib_FlushRxTsPendingPkts(ieee, pTS);
 
-	deactivate_ba_entry(ieee, pBA);
-	pBA->dialog_token = *pDialogToken;
-	pBA->ba_param_set = *pBaParamSet;
-	pBA->ba_timeout_value = *pBaTimeoutVal;
-	pBA->ba_start_seq_ctrl = *pBaStartSeqCtrl;
+	deactivate_ba_entry(ieee, pba);
+	pba->dialog_token = *pDialogToken;
+	pba->ba_param_set = *pBaParamSet;
+	pba->ba_timeout_value = *pBaTimeoutVal;
+	pba->ba_start_seq_ctrl = *pBaStartSeqCtrl;
 
 	if (ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev) ||
 	   (ieee->ht_info->iot_action & HT_IOT_ACT_ALLOW_PEER_AGG_ONE_PKT))
-		pBA->ba_param_set.field.buffer_size = 1;
+		pba->ba_param_set.field.buffer_size = 1;
 	else
-		pBA->ba_param_set.field.buffer_size = 32;
+		pba->ba_param_set.field.buffer_size = 32;
 
-	activate_ba_entry(pBA, 0);
-	rtllib_send_ADDBARsp(ieee, dst, pBA, ADDBA_STATUS_SUCCESS);
+	activate_ba_entry(pba, 0);
+	rtllib_send_ADDBARsp(ieee, dst, pba, ADDBA_STATUS_SUCCESS);
 
 	return 0;
 
@@ -464,24 +464,24 @@  int rtllib_rx_DELBA(struct rtllib_device *ieee, struct sk_buff *skb)
 void ts_init_add_ba(struct rtllib_device *ieee, struct tx_ts_record *pTS,
 		 u8 Policy, u8	bOverwritePending)
 {
-	struct ba_record *pBA = &pTS->TxPendingBARecord;
+	struct ba_record *pba = &pTS->TxPendingBARecord;
 
-	if (pBA->b_valid && !bOverwritePending)
+	if (pba->b_valid && !bOverwritePending)
 		return;
 
-	deactivate_ba_entry(ieee, pBA);
+	deactivate_ba_entry(ieee, pba);
 
-	pBA->dialog_token++;
-	pBA->ba_param_set.field.amsdu_support = 0;
-	pBA->ba_param_set.field.ba_policy = Policy;
-	pBA->ba_param_set.field.tid = pTS->TsCommonInfo.TSpec.f.TSInfo.field.ucTSID;
-	pBA->ba_param_set.field.buffer_size = 32;
-	pBA->ba_timeout_value = 0;
-	pBA->ba_start_seq_ctrl.field.seq_num = (pTS->TxCurSeq + 3) % 4096;
+	pba->dialog_token++;
+	pba->ba_param_set.field.amsdu_support = 0;
+	pba->ba_param_set.field.ba_policy = Policy;
+	pba->ba_param_set.field.tid = pTS->TsCommonInfo.TSpec.f.TSInfo.field.ucTSID;
+	pba->ba_param_set.field.buffer_size = 32;
+	pba->ba_timeout_value = 0;
+	pba->ba_start_seq_ctrl.field.seq_num = (pTS->TxCurSeq + 3) % 4096;
 
-	activate_ba_entry(pBA, BA_SETUP_TIMEOUT);
+	activate_ba_entry(pba, BA_SETUP_TIMEOUT);
 
-	rtllib_send_ADDBAReq(ieee, pTS->TsCommonInfo.Addr, pBA);
+	rtllib_send_ADDBAReq(ieee, pTS->TsCommonInfo.Addr, pba);
 }
 
 void ts_init_del_ba(struct rtllib_device *ieee,
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index 0f84ee1d418e..8f0e9dbe5b7a 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2016,7 +2016,7 @@  void ts_init_del_ba(struct rtllib_device *ieee,
 void ba_setup_time_out(struct timer_list *t);
 void tx_ba_inact_timeout(struct timer_list *t);
 void rx_ba_inact_timeout(struct timer_list *t);
-void ResetBaEntry(struct ba_record *pBA);
+void ResetBaEntry(struct ba_record *pba);
 bool GetTs(struct rtllib_device *ieee, struct ts_common_info **ppTS, u8 *Addr,
 	   u8 TID, enum tr_select TxRxSelect, bool bAddNewTs);
 void TSInitialize(struct rtllib_device *ieee);