Fix issues detected by coccinelle

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

Signed-off-by Shivani Bhardwaj <[email protected]>

---
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.

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

Signed-off-by Shivani Bhardwaj <[email protected]>

---
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.

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 39 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
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.

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

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 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 +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.

Quoted 14 lines · Shivani Bhardwaj

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

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 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.

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.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 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 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 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.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 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