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