Skip to contentSkip to conversation

Fix issues detected by coccinelle

PATCH Superseded Shivani Bhardwaj 3 participants 11 messages Oct 12, 2015 Staging

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover Fix issues detected by coccinelle

Replace appropriate functions with kzalloc(), fix NULL tests and remove unnecessary code.
After applying this patch, code becomes better.

Shivani Bhardwaj (3):
Staging: wilc1000: host_interface: Replace kmalloc with kzalloc
Staging: wilc1000: host_interface: Remove extra NULL test
Staging: wilc1000: host_interface: Remove unused code

 drivers/staging/wilc1000/host_interface.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/3 Staging: wilc1000: host_interface: Replace kmalloc with kzalloc

Replace the function kmalloc and following memset function with a single function kzalloc.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/staging/wilc1000/host_interface.c +3 -10
index bb833d3..d279eba 100644
@@ -2523,16 +2523,13 @@ static int Handle_Key(tstrWILC_WFIDrv *drvHandler,
 
 	case WPARxGtk:
 		if (pstrHostIFkeyAttr->u8KeyAction & ADDKEY_AP)	{
-			pu8keybuf = kmalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
+			pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
 			if (pu8keybuf == NULL) {
 				PRINT_ER("No buffer to send RxGTK Key\n");
 				ret = -1;
 				goto _WPARxGtk_end_case_;
 			}
 
-			memset(pu8keybuf, 0, RX_MIC_KEY_MSG_LEN);
-
-
 			/*|----------------------------------------------------------------------------|
 			 * |Sta Address | Key RSC | KeyID | Key Length | Temporal Key	| Rx Michael Key |
 			 * |------------|---------|-------|------------|---------------|----------------|
@@ -2574,16 +2571,13 @@ static int Handle_Key(tstrWILC_WFIDrv *drvHandler,
 		if (pstrHostIFkeyAttr->u8KeyAction & ADDKEY) {
 			PRINT_D(HOSTINF_DBG, "Handling group key(Rx) function\n");
 
-			pu8keybuf = kmalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
+			pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
 			if (pu8keybuf == NULL) {
 				PRINT_ER("No buffer to send RxGTK Key\n");
 				ret = -1;
 				goto _WPARxGtk_end_case_;
 			}
 
-			memset(pu8keybuf, 0, RX_MIC_KEY_MSG_LEN);
-
-
 			/*|----------------------------------------------------------------------------|
 			 * |Sta Address | Key RSC | KeyID | Key Length | Temporal Key	| Rx Michael Key |
 			 * |------------|---------|-------|------------|---------------|----------------|
@@ -6750,9 +6744,8 @@ static void *host_int_ParseJoinBssParam(tstrNetworkInfo *ptstrNetworkInfo)
 	pu8IEs = ptstrNetworkInfo->pu8IEs;
 	u16IEsLen = ptstrNetworkInfo->u16IEsLen;
 
-	pNewJoinBssParam = kmalloc(sizeof(tstrJoinBssParam), GFP_KERNEL);
+	pNewJoinBssParam = kzalloc(sizeof(tstrJoinBssParam), GFP_KERNEL);
 	if (pNewJoinBssParam != NULL) {
-		memset(pNewJoinBssParam, 0, sizeof(tstrJoinBssParam));
 		pNewJoinBssParam->dtim_period = ptstrNetworkInfo->u8DtimPeriod;
 		pNewJoinBssParam->beacon_period = ptstrNetworkInfo->u16BeaconPeriod;
 		pNewJoinBssParam->cap_info = ptstrNetworkInfo->u16CapInfo;
Signature · 1 line
2.1.0
#2 lore raw

Re: [Outreachy kernel] [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc

Quoted 3 lines · Shivani Bhardwaj

Replace the function kmalloc and following memset function with a single function kzalloc.
Issue found using coccinelle.

Hello Shivani,
This was done by me in a patch series on 9th Oct. Since it hasn't been
applied yet, you probably missed it :)

Shraddha

Quoted 67 lines

---
drivers/staging/wilc1000/host_interface.c | 13 +++----------

1 file changed, 3 insertions(+), 10 deletions(-)
drivers/staging/wilc1000/host_interface.c +3 -10
index bb833d3..d279eba 100644
@@ -2523,16 +2523,13 @@ static int Handle_Key(tstrWILC_WFIDrv *drvHandler,
 
 	case WPARxGtk:
 		if (pstrHostIFkeyAttr->u8KeyAction & ADDKEY_AP)	{
-			pu8keybuf = kmalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
+			pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
 			if (pu8keybuf == NULL) {
 				PRINT_ER("No buffer to send RxGTK Key\n");
 				ret = -1;
 				goto _WPARxGtk_end_case_;
 			}
 
-			memset(pu8keybuf, 0, RX_MIC_KEY_MSG_LEN);
-
-
 			/*|----------------------------------------------------------------------------|
 			 * |Sta Address | Key RSC | KeyID | Key Length | Temporal Key	| Rx Michael Key |
 			 * |------------|---------|-------|------------|---------------|----------------|
@@ -2574,16 +2571,13 @@ static int Handle_Key(tstrWILC_WFIDrv *drvHandler,
 		if (pstrHostIFkeyAttr->u8KeyAction & ADDKEY) {
 			PRINT_D(HOSTINF_DBG, "Handling group key(Rx) function\n");
 
-			pu8keybuf = kmalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
+			pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
 			if (pu8keybuf == NULL) {
 				PRINT_ER("No buffer to send RxGTK Key\n");
 				ret = -1;
 				goto _WPARxGtk_end_case_;
 			}
 
-			memset(pu8keybuf, 0, RX_MIC_KEY_MSG_LEN);
-
-
 			/*|----------------------------------------------------------------------------|
 			 * |Sta Address | Key RSC | KeyID | Key Length | Temporal Key	| Rx Michael Key |
 			 * |------------|---------|-------|------------|---------------|----------------|
@@ -6750,9 +6744,8 @@ static void *host_int_ParseJoinBssParam(tstrNetworkInfo *ptstrNetworkInfo)
 	pu8IEs = ptstrNetworkInfo->pu8IEs;
 	u16IEsLen = ptstrNetworkInfo->u16IEsLen;
 
-	pNewJoinBssParam = kmalloc(sizeof(tstrJoinBssParam), GFP_KERNEL);
+	pNewJoinBssParam = kzalloc(sizeof(tstrJoinBssParam), GFP_KERNEL);
 	if (pNewJoinBssParam != NULL) {
-		memset(pNewJoinBssParam, 0, sizeof(tstrJoinBssParam));
 		pNewJoinBssParam->dtim_period = ptstrNetworkInfo->u8DtimPeriod;
 		pNewJoinBssParam->beacon_period = ptstrNetworkInfo->u16BeaconPeriod;
 		pNewJoinBssParam->cap_info = ptstrNetworkInfo->u16CapInfo;
Signature · 8 lines
2.1.0

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/eac47047141a3ed75e04ca4cfb1fece8a8ef349f.1444655924.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
#3 lore raw

PATCH 2/3 Staging: wilc1000: host_interface: Remove extra NULL test

In reply to Shivani Bhardwaj

Remove NULL test statement as it is preceded by another NULL test on the same variable in the code.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/staging/wilc1000/host_interface.c +2 -5
index d279eba..b6eea9a 100644
@@ -6038,11 +6038,8 @@ s32 host_int_deinit(tstrWILC_WFIDrv *hWFIDrv)
 	if (ret)
 		s32Error = -ENOENT;
 
-	if (pstrWFIDrv != NULL) {
-		kfree(pstrWFIDrv);
-		/* pstrWFIDrv=NULL; */
-
-	}
+	kfree(pstrWFIDrv);
+	/* pstrWFIDrv=NULL; */
 
 	clients_count--; /* Decrease number of created entities */
 	terminated_handle = NULL;
Signature · 1 line
2.1.0
#4 lore raw

Re: [Outreachy kernel] [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test

Quoted 3 lines · Shivani Bhardwaj

Remove NULL test statement as it is preceded by another NULL test on the same variable in the code.
Issue found using coccinelle.

While you are at it, you might want to consider taking a closer look at this file. There are other instances of unnecessary null test which can be found using Coccinelle.I didn't work on this one since you already are :)

Shraddha

Quoted 34 lines

---
drivers/staging/wilc1000/host_interface.c | 7 ++-----

1 file changed, 2 insertions(+), 5 deletions(-)
drivers/staging/wilc1000/host_interface.c +2 -5
index d279eba..b6eea9a 100644
@@ -6038,11 +6038,8 @@ s32 host_int_deinit(tstrWILC_WFIDrv *hWFIDrv)
 	if (ret)
 		s32Error = -ENOENT;
 
-	if (pstrWFIDrv != NULL) {
-		kfree(pstrWFIDrv);
-		/* pstrWFIDrv=NULL; */
-
-	}
+	kfree(pstrWFIDrv);
+	/* pstrWFIDrv=NULL; */
 
 	clients_count--; /* Decrease number of created entities */
 	terminated_handle = NULL;
Signature · 8 lines
2.1.0

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/c122920d4c47aae820bb3a350cdb8511376b0c3c.1444655924.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
#5 lore raw

Re: [Outreachy kernel] [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test

Quoted 9 lines · Shraddha Barke
Quoted 3 lines · Shivani Bhardwaj

Remove NULL test statement as it is preceded by another NULL test on the same variable in the code.
Issue found using coccinelle.

Quoted 5 lines

While you are at it, you might want to consider taking a closer look at this file. There are other instances of unnecessary null test which can be found using Coccinelle.I didn't work on this one since you already are :)

Shraddha

Thanks! Are you writing your own cocci scripts? I used the one made available by Julia, it finds only one.

Quoted 40 lines
Quoted 39 lines

---
drivers/staging/wilc1000/host_interface.c | 7 ++-----

1 file changed, 2 insertions(+), 5 deletions(-)
drivers/staging/wilc1000/host_interface.c +2 -6
index d279eba..b6eea9a 100644
@@ -6038,11 +6038,8 @@ s32 host_int_deinit(tstrWILC_WFIDrv *hWFIDrv)
        if (ret)
                s32Error = -ENOENT;
 
-       if (pstrWFIDrv != NULL) {
-               kfree(pstrWFIDrv);
-               /* pstrWFIDrv=NULL; */
-
-       }
+       kfree(pstrWFIDrv);
+       /* pstrWFIDrv=NULL; */
 
        clients_count--; /* Decrease number of created entities */
        terminated_handle = NULL;
--

2.1.0

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected]. To view this discussion on the web visit
https://groups.google.com/d/msgid/outreachy-kernel/c122920d4c47aae820bb3a350cdb8511376b0c3c.1444655924.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

#6 lore raw

PATCH 3/3 Staging: wilc1000: host_interface: Remove unused code

In reply to Shivani Bhardwaj

Remove declaration and initialization of variables that are not used anywhere in the code.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/staging/wilc1000/host_interface.c +0 -4
index b6eea9a..9e8bbc1 100644
@@ -4894,7 +4894,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
 	s32 s32Error = 0;
 	tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
 	struct host_if_msg msg;
-	tenuScanConnTimer enuScanConnTimer;
 
 	if (pstrWFIDrv == NULL || pfConnectResult == NULL) {
 		s32Error = -EFAULT;
@@ -4957,7 +4956,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
 		return -EFAULT;
 	}
 
-	enuScanConnTimer = CONNECT_TIMER;
 	pstrWFIDrv->hConnectTimer.data = (unsigned long)hWFIDrv;
 	mod_timer(&pstrWFIDrv->hConnectTimer,
 		  jiffies + msecs_to_jiffies(HOST_IF_CONNECT_TIMEOUT));
@@ -5560,7 +5558,6 @@ s32 host_int_scan(tstrWILC_WFIDrv *hWFIDrv, u8 u8ScanSource,
 	s32 s32Error = 0;
 	tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
 	struct host_if_msg msg;
-	tenuScanConnTimer enuScanConnTimer;
 
 	if (pstrWFIDrv == NULL || ScanResult == NULL) {
 		PRINT_ER("pstrWFIDrv or ScanResult = NULL\n");
@@ -5602,7 +5599,6 @@ s32 host_int_scan(tstrWILC_WFIDrv *hWFIDrv, u8 u8ScanSource,
 		return -EINVAL;
 	}
 
-	enuScanConnTimer = SCAN_TIMER;
 	PRINT_D(HOSTINF_DBG, ">> Starting the SCAN timer\n");
 	pstrWFIDrv->hScanTimer.data = (unsigned long)hWFIDrv;
 	mod_timer(&pstrWFIDrv->hScanTimer,
Signature · 1 line
2.1.0
#7 lore raw

Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code

Quoted 14 lines · Shivani Bhardwaj

Remove declaration and initialization of variables that are not used anywhere in the code.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/staging/wilc1000/host_interface.c +0 -0
index b6eea9a..9e8bbc1 100644

drivers/staging/wilc1000/host_interface.c s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid, · line 4897

Quoted 4 lines
drivers/staging/wilc1000/host_interface.c +0 -1
@@ -4894,7 +4894,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
 	s32 s32Error = 0;
 	tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
 	struct host_if_msg msg;
-	tenuScanConnTimer enuScanConnTimer;

Got grep shows me only three uses of "tenuScanConnTimer". One is the typedef, and the other two are removed by your patch. So you could redo the patch and get rid of the typedef as well.

Looking a bit more at the code, I see the following, which looks strange:

tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;

As far as I can tell, hWFIDrv already has type tstrWILC_WFIDrv, so the cast is not necessary. And it seems strange to have the two tests, first

if (pstrWFIDrv == NULL || pfConnectResult == NULL)

and then

if (hWFIDrv == NULL)

because the second one tests a subset of what the first one tests.

Greg would also say that we don't need the type names embedded in the variable names either, but that would be a lot to change, especially since it impacts the field names too.

In Coccinelle, you can make an expression metavariable that matches expressions of a particular type. So

@@
type T;
T e;
identifier x;
@@

* T x = (T)e;

will highlight initializations with useless casts.

julia

Quoted 37 lines
drivers/staging/wilc1000/host_interface.c +0 -4
 	if (pstrWFIDrv == NULL || pfConnectResult == NULL) {
 		s32Error = -EFAULT;
@@ -4957,7 +4956,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
 		return -EFAULT;
 	}
 
-	enuScanConnTimer = CONNECT_TIMER;
 	pstrWFIDrv->hConnectTimer.data = (unsigned long)hWFIDrv;
 	mod_timer(&pstrWFIDrv->hConnectTimer,
 		  jiffies + msecs_to_jiffies(HOST_IF_CONNECT_TIMEOUT));
@@ -5560,7 +5558,6 @@ s32 host_int_scan(tstrWILC_WFIDrv *hWFIDrv, u8 u8ScanSource,
 	s32 s32Error = 0;
 	tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
 	struct host_if_msg msg;
-	tenuScanConnTimer enuScanConnTimer;
 
 	if (pstrWFIDrv == NULL || ScanResult == NULL) {
 		PRINT_ER("pstrWFIDrv or ScanResult = NULL\n");
@@ -5602,7 +5599,6 @@ s32 host_int_scan(tstrWILC_WFIDrv *hWFIDrv, u8 u8ScanSource,
 		return -EINVAL;
 	}
 
-	enuScanConnTimer = SCAN_TIMER;
 	PRINT_D(HOSTINF_DBG, ">> Starting the SCAN timer\n");
 	pstrWFIDrv->hScanTimer.data = (unsigned long)hWFIDrv;
 	mod_timer(&pstrWFIDrv->hScanTimer,
--

2.1.0

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d8f8cff7018b0c9905df40eaa754b53e8164324e.1444655924.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

#8 lore raw

Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code

Quoted 53 lines · Julia Lawall
Quoted 14 lines · Shivani Bhardwaj

Remove declaration and initialization of variables that are not used anywhere in the code.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/staging/wilc1000/host_interface.c +0 -0
index b6eea9a..9e8bbc1 100644

drivers/staging/wilc1000/host_interface.c s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid, · line 4897

Quoted 4 lines
drivers/staging/wilc1000/host_interface.c +0 -1
@@ -4894,7 +4894,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
      s32 s32Error = 0;
      tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
      struct host_if_msg msg;
-     tenuScanConnTimer enuScanConnTimer;

Got grep shows me only three uses of "tenuScanConnTimer". One is the typedef, and the other two are removed by your patch. So you could redo the patch and get rid of the typedef as well.

Looking a bit more at the code, I see the following, which looks strange:

tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;

As far as I can tell, hWFIDrv already has type tstrWILC_WFIDrv, so the cast is not necessary. And it seems strange to have the two tests, first

if (pstrWFIDrv == NULL || pfConnectResult == NULL)

and then

if (hWFIDrv == NULL)

because the second one tests a subset of what the first one tests.

Greg would also say that we don't need the type names embedded in the variable names either, but that would be a lot to change, especially since it impacts the field names too.

In Coccinelle, you can make an expression metavariable that matches expressions of a particular type. So

@@
type T;
T e;
identifier x;
@@

Quoted 6 lines

* T x = (T)e;

will highlight initializations with useless casts.

julia

Thanks so much, Julia. I'm applying all of this. Thank you for the coccinelle script. Also, I want to know that I've been notified about one of the patch of this series that it was sent before and has not been applied. So, should I send an entirely new patch series removing that one or even parts of the series can be applied to the tree?

Quoted 37 lines
Quoted 37 lines
drivers/staging/wilc1000/host_interface.c +0 -4
      if (pstrWFIDrv == NULL || pfConnectResult == NULL) {
              s32Error = -EFAULT;
@@ -4957,7 +4956,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
              return -EFAULT;
      }
 
-     enuScanConnTimer = CONNECT_TIMER;
      pstrWFIDrv->hConnectTimer.data = (unsigned long)hWFIDrv;
      mod_timer(&pstrWFIDrv->hConnectTimer,
                jiffies + msecs_to_jiffies(HOST_IF_CONNECT_TIMEOUT));
@@ -5560,7 +5558,6 @@ s32 host_int_scan(tstrWILC_WFIDrv *hWFIDrv, u8 u8ScanSource,
      s32 s32Error = 0;
      tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
      struct host_if_msg msg;
-     tenuScanConnTimer enuScanConnTimer;
 
      if (pstrWFIDrv == NULL || ScanResult == NULL) {
              PRINT_ER("pstrWFIDrv or ScanResult = NULL\n");
@@ -5602,7 +5599,6 @@ s32 host_int_scan(tstrWILC_WFIDrv *hWFIDrv, u8 u8ScanSource,
              return -EINVAL;
      }
 
-     enuScanConnTimer = SCAN_TIMER;
      PRINT_D(HOSTINF_DBG, ">> Starting the SCAN timer\n");
      pstrWFIDrv->hScanTimer.data = (unsigned long)hWFIDrv;
      mod_timer(&pstrWFIDrv->hScanTimer,
--

2.1.0

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d8f8cff7018b0c9905df40eaa754b53e8164324e.1444655924.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

#9 lore raw

Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code

Quoted 58 lines · Shivani Bhardwaj
Quoted 57 lines · Julia Lawall
Quoted 14 lines · Shivani Bhardwaj

Remove declaration and initialization of variables that are not used anywhere in the code.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/staging/wilc1000/host_interface.c +0 -0
index b6eea9a..9e8bbc1 100644

drivers/staging/wilc1000/host_interface.c s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid, · line 4897

Quoted 4 lines
drivers/staging/wilc1000/host_interface.c +0 -1
@@ -4894,7 +4894,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
      s32 s32Error = 0;
      tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
      struct host_if_msg msg;
-     tenuScanConnTimer enuScanConnTimer;

Got grep shows me only three uses of "tenuScanConnTimer". One is the typedef, and the other two are removed by your patch. So you could redo the patch and get rid of the typedef as well.

Looking a bit more at the code, I see the following, which looks strange:

tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;

As far as I can tell, hWFIDrv already has type tstrWILC_WFIDrv, so the cast is not necessary. And it seems strange to have the two tests, first

if (pstrWFIDrv == NULL || pfConnectResult == NULL)

and then

if (hWFIDrv == NULL)

because the second one tests a subset of what the first one tests.

Greg would also say that we don't need the type names embedded in the variable names either, but that would be a lot to change, especially since it impacts the field names too.

In Coccinelle, you can make an expression metavariable that matches expressions of a particular type. So

@@
type T;
T e;
identifier x;
@@

* T x = (T)e;

will highlight initializations with useless casts.

Quoted 8 lines
Quoted 2 lines

julia

Thanks so much, Julia. I'm applying all of this. Thank you for the coccinelle script. Also, I want to know that I've been notified about one of the patch of this series that it was sent before and has not been applied. So, should I send an entirely new patch series removing that one or even parts of the series can be applied to the tree?

Since it is all on one file, maybe the best is to send a v2 of the series with the redundant patch removed, the patch that removes the variable declaration updated to remove the type too, and as much of the rest as you want to do in the short term.

julia

#10 lore raw

Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code

Quoted 71 lines · Julia Lawall
Quoted 58 lines · Shivani Bhardwaj
Quoted 57 lines · Julia Lawall
Quoted 14 lines · Shivani Bhardwaj

Remove declaration and initialization of variables that are not used anywhere in the code.
Issue found using coccinelle.

---
 drivers/staging/wilc1000/host_interface.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/staging/wilc1000/host_interface.c +0 -0
index b6eea9a..9e8bbc1 100644

drivers/staging/wilc1000/host_interface.c s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid, · line 4897

Quoted 4 lines
drivers/staging/wilc1000/host_interface.c +0 -1
@@ -4894,7 +4894,6 @@ s32 host_int_set_join_req(tstrWILC_WFIDrv *hWFIDrv, u8 *pu8bssid,
      s32 s32Error = 0;
      tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;
      struct host_if_msg msg;
-     tenuScanConnTimer enuScanConnTimer;

Got grep shows me only three uses of "tenuScanConnTimer". One is the typedef, and the other two are removed by your patch. So you could redo the patch and get rid of the typedef as well.

Looking a bit more at the code, I see the following, which looks strange:

tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv;

As far as I can tell, hWFIDrv already has type tstrWILC_WFIDrv, so the cast is not necessary. And it seems strange to have the two tests, first

if (pstrWFIDrv == NULL || pfConnectResult == NULL)

and then

if (hWFIDrv == NULL)

because the second one tests a subset of what the first one tests.

Greg would also say that we don't need the type names embedded in the variable names either, but that would be a lot to change, especially since it impacts the field names too.

In Coccinelle, you can make an expression metavariable that matches expressions of a particular type. So

@@
type T;
T e;
identifier x;
@@

* T x = (T)e;

will highlight initializations with useless casts.

Quoted 8 lines
Quoted 2 lines

julia

Thanks so much, Julia. I'm applying all of this. Thank you for the coccinelle script. Also, I want to know that I've been notified about one of the patch of this series that it was sent before and has not been applied. So, should I send an entirely new patch series removing that one or even parts of the series can be applied to the tree?

Quoted 6 lines

Since it is all on one file, maybe the best is to send a v2 of the series with the redundant patch removed, the patch that removes the variable declaration updated to remove the type too, and as much of the rest as you want to do in the short term.

julia

OK. Thanks. I'm sending v2.