All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix issues detected by coccinelle
@ 2015-10-12 13:34 Shivani Bhardwaj
  2015-10-12 13:35 ` [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc Shivani Bhardwaj
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 13:34 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: outreachy-kernel

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

-- 
2.1.0



^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc
  2015-10-12 13:34 [PATCH 0/3] Fix issues detected by coccinelle Shivani Bhardwaj
@ 2015-10-12 13:35 ` Shivani Bhardwaj
  2015-10-12 13:45   ` [Outreachy kernel] " Shraddha Barke
  2015-10-12 13:35 ` [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test Shivani Bhardwaj
  2015-10-12 13:36 ` [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code Shivani Bhardwaj
  2 siblings, 1 reply; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 13:35 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: outreachy-kernel

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

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/wilc1000/host_interface.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index bb833d3..d279eba 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -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;
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test
  2015-10-12 13:34 [PATCH 0/3] Fix issues detected by coccinelle Shivani Bhardwaj
  2015-10-12 13:35 ` [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc Shivani Bhardwaj
@ 2015-10-12 13:35 ` Shivani Bhardwaj
  2015-10-12 15:42   ` [Outreachy kernel] " Shraddha Barke
  2015-10-12 13:36 ` [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code Shivani Bhardwaj
  2 siblings, 1 reply; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 13:35 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: outreachy-kernel

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 <shivanib134@gmail.com>
---
 drivers/staging/wilc1000/host_interface.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index d279eba..b6eea9a 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -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



^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code
  2015-10-12 13:34 [PATCH 0/3] Fix issues detected by coccinelle Shivani Bhardwaj
  2015-10-12 13:35 ` [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc Shivani Bhardwaj
  2015-10-12 13:35 ` [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test Shivani Bhardwaj
@ 2015-10-12 13:36 ` Shivani Bhardwaj
  2015-10-12 13:54   ` [Outreachy kernel] " Julia Lawall
  2 siblings, 1 reply; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 13:36 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: outreachy-kernel

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

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/wilc1000/host_interface.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index b6eea9a..9e8bbc1 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -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,
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc
  2015-10-12 13:35 ` [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc Shivani Bhardwaj
@ 2015-10-12 13:45   ` Shraddha Barke
  0 siblings, 0 replies; 11+ messages in thread
From: Shraddha Barke @ 2015-10-12 13:45 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel



On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:

> 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
>
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
> drivers/staging/wilc1000/host_interface.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index bb833d3..d279eba 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -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;
> -- 
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code
  2015-10-12 13:36 ` [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code Shivani Bhardwaj
@ 2015-10-12 13:54   ` Julia Lawall
  2015-10-12 14:21     ` Shivani Bhardwaj
  0 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2015-10-12 13:54 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel

On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:

> Remove declaration and initialization of variables that are not used
> anywhere in the code.
> Issue found using coccinelle.
>
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
>  drivers/staging/wilc1000/host_interface.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index b6eea9a..9e8bbc1 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -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

>
>  	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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code
  2015-10-12 13:54   ` [Outreachy kernel] " Julia Lawall
@ 2015-10-12 14:21     ` Shivani Bhardwaj
  2015-10-12 14:27       ` Julia Lawall
  0 siblings, 1 reply; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 14:21 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Mon, Oct 12, 2015 at 7:24 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:
>
>> Remove declaration and initialization of variables that are not used
>> anywhere in the code.
>> Issue found using coccinelle.
>>
>> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> ---
>>  drivers/staging/wilc1000/host_interface.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
>> index b6eea9a..9e8bbc1 100644
>> --- a/drivers/staging/wilc1000/host_interface.c
>> +++ b/drivers/staging/wilc1000/host_interface.c
>> @@ -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
>

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?

>>
>>       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 outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> 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.
>>


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code
  2015-10-12 14:21     ` Shivani Bhardwaj
@ 2015-10-12 14:27       ` Julia Lawall
  2015-10-12 15:24         ` Shivani Bhardwaj
  0 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2015-10-12 14:27 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel



On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:

> On Mon, Oct 12, 2015 at 7:24 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:
> >
> >> Remove declaration and initialization of variables that are not used
> >> anywhere in the code.
> >> Issue found using coccinelle.
> >>
> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> >> ---
> >>  drivers/staging/wilc1000/host_interface.c | 4 ----
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> >> index b6eea9a..9e8bbc1 100644
> >> --- a/drivers/staging/wilc1000/host_interface.c
> >> +++ b/drivers/staging/wilc1000/host_interface.c
> >> @@ -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
> >
>
> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code
  2015-10-12 14:27       ` Julia Lawall
@ 2015-10-12 15:24         ` Shivani Bhardwaj
  0 siblings, 0 replies; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 15:24 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Mon, Oct 12, 2015 at 7:57 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:
>
>> On Mon, Oct 12, 2015 at 7:24 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:
>> >
>> >> Remove declaration and initialization of variables that are not used
>> >> anywhere in the code.
>> >> Issue found using coccinelle.
>> >>
>> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> >> ---
>> >>  drivers/staging/wilc1000/host_interface.c | 4 ----
>> >>  1 file changed, 4 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
>> >> index b6eea9a..9e8bbc1 100644
>> >> --- a/drivers/staging/wilc1000/host_interface.c
>> >> +++ b/drivers/staging/wilc1000/host_interface.c
>> >> @@ -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
>> >
>>
>> 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

OK. Thanks. I'm sending v2.


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test
  2015-10-12 13:35 ` [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test Shivani Bhardwaj
@ 2015-10-12 15:42   ` Shraddha Barke
  2015-10-12 15:48     ` Shivani Bhardwaj
  0 siblings, 1 reply; 11+ messages in thread
From: Shraddha Barke @ 2015-10-12 15:42 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel



On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:

> 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
>
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
> drivers/staging/wilc1000/host_interface.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index d279eba..b6eea9a 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test
  2015-10-12 15:42   ` [Outreachy kernel] " Shraddha Barke
@ 2015-10-12 15:48     ` Shivani Bhardwaj
  0 siblings, 0 replies; 11+ messages in thread
From: Shivani Bhardwaj @ 2015-10-12 15:48 UTC (permalink / raw)
  To: Shraddha Barke; +Cc: outreachy-kernel

On Mon, Oct 12, 2015 at 9:12 PM, Shraddha Barke <shraddha.6596@gmail.com> wrote:
>
>
> On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:
>
>> 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

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

>>
>>
>> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> ---
>> drivers/staging/wilc1000/host_interface.c | 7 ++-----
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/wilc1000/host_interface.c
>> b/drivers/staging/wilc1000/host_interface.c
>> index d279eba..b6eea9a 100644
>> --- a/drivers/staging/wilc1000/host_interface.c
>> +++ b/drivers/staging/wilc1000/host_interface.c
>> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> 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.
>>
>


^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-10-12 15:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12 13:34 [PATCH 0/3] Fix issues detected by coccinelle Shivani Bhardwaj
2015-10-12 13:35 ` [PATCH 1/3] Staging: wilc1000: host_interface: Replace kmalloc with kzalloc Shivani Bhardwaj
2015-10-12 13:45   ` [Outreachy kernel] " Shraddha Barke
2015-10-12 13:35 ` [PATCH 2/3] Staging: wilc1000: host_interface: Remove extra NULL test Shivani Bhardwaj
2015-10-12 15:42   ` [Outreachy kernel] " Shraddha Barke
2015-10-12 15:48     ` Shivani Bhardwaj
2015-10-12 13:36 ` [PATCH 3/3] Staging: wilc1000: host_interface: Remove unused code Shivani Bhardwaj
2015-10-12 13:54   ` [Outreachy kernel] " Julia Lawall
2015-10-12 14:21     ` Shivani Bhardwaj
2015-10-12 14:27       ` Julia Lawall
2015-10-12 15:24         ` Shivani Bhardwaj

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.