* [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci
@ 2019-03-17 0:32 Madhumitha Prabakaran
2019-03-17 0:32 ` [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names Madhumitha Prabakaran
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
This patchset does tasks as below:
1) Drop wrappers and change function names
2) Convert type of local variables and return type
3) Change values of error codes in functions
4) Remove unnecessary function declarations
5) Replace error codes with values
6) Remove extern and change return type
7) Add missing NULL check for kmalloc
All these patches are relevant to an issue suggested by Coccinelle using
ret.cocci.
Madhumitha Prabakaran (7):
Staging: rtl8723bs: Drop wrappers and change function names
Staging: rtl8723bs: Convert type of local variables and return types
Staging: rtl8723bs: Change values of error codes in functions
Staging: rtl8723bs: Remove unnecessary function declarations
Staging: rtl8723bs: Replace error codes with values
Staging: rtl8723bs: Remove extern and change return type
Staging: rtl8723bs: Add missing NULL check for kmalloc
drivers/staging/rtl8723bs/core/rtw_cmd.c | 30 +++++--------------
drivers/staging/rtl8723bs/include/cmd_osdep.h | 3 --
drivers/staging/rtl8723bs/include/rtw_cmd.h | 4 +--
drivers/staging/rtl8723bs/os_dep/os_intfs.c | 4 +--
4 files changed, 12 insertions(+), 29 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 10:59 ` Greg KH
2019-03-17 0:32 ` [PATCH 2/7] Staging: rtl8723bs: Convert type of local variables and return types Madhumitha Prabakaran
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
- Drop wrappers rtw_init_cmd_priv and rtw_init_evt_priv,
as their only purpose is to call other functions
_rtw_init_cmd_priv and _rtw_init_evt_priv, respectively.
- Change function names
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 91520ca3bbad..793bc505fa9c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -162,7 +162,7 @@ Caller and the rtw_cmd_thread can protect cmd_q by spin_lock.
No irqsave is necessary.
*/
-sint _rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
+sint rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
{
sint res = _SUCCESS;
@@ -201,7 +201,7 @@ sint _rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
}
static void c2h_wk_callback(_workitem *work);
-sint _rtw_init_evt_priv(struct evt_priv *pevtpriv)
+sint rtw_init_evt_priv(struct evt_priv *pevtpriv)
{
sint res = _SUCCESS;
@@ -295,22 +295,6 @@ struct cmd_obj *_rtw_dequeue_cmd(struct __queue *queue)
return obj;
}
-u32 rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
-{
- u32 res;
-
- res = _rtw_init_cmd_priv(pcmdpriv);
- return res;
-}
-
-u32 rtw_init_evt_priv(struct evt_priv *pevtpriv)
-{
- int res;
-
- res = _rtw_init_evt_priv(pevtpriv);
- return res;
-}
-
void rtw_free_evt_priv(struct evt_priv *pevtpriv)
{
RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, ("rtw_free_evt_priv\n"));
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/7] Staging: rtl8723bs: Convert type of local variables and return types
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
2019-03-17 0:32 ` [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 6:42 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 3/7] Staging: rtl8723bs: Change values of error codes in functions Madhumitha Prabakaran
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
Convert types from sint to int for local variable 'res' and return types
of function 'rtw_init_cmd_pri' and 'rtw_init_evt_priv', as function's
return types are defined for error codes _SUCCESS and _FAIL.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 793bc505fa9c..ab450f717f34 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -162,9 +162,9 @@ Caller and the rtw_cmd_thread can protect cmd_q by spin_lock.
No irqsave is necessary.
*/
-sint rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
+int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
{
- sint res = _SUCCESS;
+ int res = _SUCCESS;
init_completion(&pcmdpriv->cmd_queue_comp);
init_completion(&pcmdpriv->terminate_cmdthread_comp);
@@ -201,9 +201,9 @@ sint rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
}
static void c2h_wk_callback(_workitem *work);
-sint rtw_init_evt_priv(struct evt_priv *pevtpriv)
+int rtw_init_evt_priv(struct evt_priv *pevtpriv)
{
- sint res = _SUCCESS;
+ int res = _SUCCESS;
/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
atomic_set(&pevtpriv->event_seq, 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/7] Staging: rtl8723bs: Change values of error codes in functions
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
2019-03-17 0:32 ` [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names Madhumitha Prabakaran
2019-03-17 0:32 ` [PATCH 2/7] Staging: rtl8723bs: Convert type of local variables and return types Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 6:38 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 4/7] Staging: rtl8723bs: Remove unnecessary function declarations Madhumitha Prabakaran
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
Change values of error codes _SUCCESS and _FAIL in function
rtw_init_cmd_priv and rtw_init_evt_priv, as the code need to be
compatible with kernel, _SUCCESS should be 0 and _FAIL should be -ENOMEM
(as in this case, these are memory allocating functions).
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index ab450f717f34..2890e12f6fb1 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -162,9 +162,9 @@ Caller and the rtw_cmd_thread can protect cmd_q by spin_lock.
No irqsave is necessary.
*/
-int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
+int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
{
- int res = _SUCCESS;
+ int res = 0;
init_completion(&pcmdpriv->cmd_queue_comp);
init_completion(&pcmdpriv->terminate_cmdthread_comp);
@@ -178,7 +178,7 @@ int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
pcmdpriv->cmd_allocated_buf = rtw_zmalloc(MAX_CMDSZ + CMDBUFF_ALIGN_SZ);
if (pcmdpriv->cmd_allocated_buf == NULL) {
- res = _FAIL;
+ res = -ENOMEM;
goto exit;
}
@@ -187,7 +187,7 @@ int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
pcmdpriv->rsp_allocated_buf = rtw_zmalloc(MAX_RSPSZ + 4);
if (pcmdpriv->rsp_allocated_buf == NULL) {
- res = _FAIL;
+ res = -ENOMEM;
goto exit;
}
@@ -203,7 +203,7 @@ int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
static void c2h_wk_callback(_workitem *work);
int rtw_init_evt_priv(struct evt_priv *pevtpriv)
{
- int res = _SUCCESS;
+ int res = 0;
/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
atomic_set(&pevtpriv->event_seq, 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/7] Staging: rtl8723bs: Remove unnecessary function declarations
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
` (2 preceding siblings ...)
2019-03-17 0:32 ` [PATCH 3/7] Staging: rtl8723bs: Change values of error codes in functions Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 6:34 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 5/7] Staging: rtl8723bs: Replace error codes with values Madhumitha Prabakaran
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
Remove unnecessary function declarations, as it is never used in a
kernel tree.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/include/cmd_osdep.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/cmd_osdep.h b/drivers/staging/rtl8723bs/include/cmd_osdep.h
index 0749936df032..6940d86e93cf 100644
--- a/drivers/staging/rtl8723bs/include/cmd_osdep.h
+++ b/drivers/staging/rtl8723bs/include/cmd_osdep.h
@@ -7,9 +7,6 @@
#ifndef __CMD_OSDEP_H_
#define __CMD_OSDEP_H_
-
-extern sint _rtw_init_cmd_priv (struct cmd_priv *pcmdpriv);
-extern sint _rtw_init_evt_priv(struct evt_priv *pevtpriv);
extern void _rtw_free_evt_priv (struct evt_priv *pevtpriv);
extern void _rtw_free_cmd_priv (struct cmd_priv *pcmdpriv);
extern sint _rtw_enqueue_cmd(struct __queue *queue, struct cmd_obj *obj);
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/7] Staging: rtl8723bs: Replace error codes with values
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
` (3 preceding siblings ...)
2019-03-17 0:32 ` [PATCH 4/7] Staging: rtl8723bs: Remove unnecessary function declarations Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 6:33 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type Madhumitha Prabakaran
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
Replace error codes with values in order to match return types of called
functions rtw_init_cmd_priv and rtw_init_evt_priv.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/os_dep/os_intfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index 143e3f9b31aa..6b3def0397f0 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -757,7 +757,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
rtw_init_hal_com_default_value(padapter);
- if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == _FAIL) {
+ if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == -ENOMEM) {
RT_TRACE(_module_os_intfs_c_, _drv_err_, ("\n Can't init cmd_priv\n"));
ret8 = _FAIL;
goto exit;
@@ -765,7 +765,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
padapter->cmdpriv.padapter = padapter;
- if ((rtw_init_evt_priv(&padapter->evtpriv)) == _FAIL) {
+ if ((rtw_init_evt_priv(&padapter->evtpriv)) == -ENOMEM) {
RT_TRACE(_module_os_intfs_c_, _drv_err_, ("\n Can't init evt_priv\n"));
ret8 = _FAIL;
goto exit;
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
` (4 preceding siblings ...)
2019-03-17 0:32 ` [PATCH 5/7] Staging: rtl8723bs: Replace error codes with values Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 6:29 ` [Outreachy kernel] " Julia Lawall
2019-03-17 6:30 ` Julia Lawall
2019-03-17 0:32 ` [PATCH 7/7] Staging: rtl8723bs: Add missing NULL check for kmalloc Madhumitha Prabakaran
2019-03-17 6:43 ` [Outreachy kernel] [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Julia Lawall
7 siblings, 2 replies; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
- Remove externs and change return types in function declarations, to
compensate change made in function definitions.
- Remove space after functions and before ( in function declarations.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/include/rtw_cmd.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
index 299b55538788..96d71b70f476 100644
--- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
+++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
@@ -129,10 +129,10 @@ extern void rtw_free_cmd_obj(struct cmd_obj *pcmd);
void rtw_stop_cmd_thread(struct adapter *adapter);
int rtw_cmd_thread(void *context);
-extern u32 rtw_init_cmd_priv (struct cmd_priv *pcmdpriv);
+int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv);
extern void rtw_free_cmd_priv (struct cmd_priv *pcmdpriv);
-extern u32 rtw_init_evt_priv (struct evt_priv *pevtpriv);
+int rtw_init_evt_priv(struct evt_priv *pevtpriv);
extern void rtw_free_evt_priv (struct evt_priv *pevtpriv);
extern void rtw_evt_notify_isr(struct evt_priv *pevtpriv);
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/7] Staging: rtl8723bs: Add missing NULL check for kmalloc
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
` (5 preceding siblings ...)
2019-03-17 0:32 ` [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type Madhumitha Prabakaran
@ 2019-03-17 0:32 ` Madhumitha Prabakaran
2019-03-17 10:55 ` Greg KH
2019-03-17 6:43 ` [Outreachy kernel] [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Julia Lawall
7 siblings, 1 reply; 17+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-17 0:32 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
Include missing NULL check for kmalloc in function rtw_init_evt_priv.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 2890e12f6fb1..4ea6483f4634 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -213,6 +213,8 @@ int rtw_init_evt_priv(struct evt_priv *pevtpriv)
pevtpriv->c2h_wk_alive = false;
pevtpriv->c2h_queue = rtw_cbuf_alloc(C2H_QUEUE_MAX_LEN+1);
+ if (!pevtpriv->c2h_queue)
+ return -ENOMEM;
return res;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type
2019-03-17 0:32 ` [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type Madhumitha Prabakaran
@ 2019-03-17 6:29 ` Julia Lawall
2019-03-17 6:30 ` Julia Lawall
1 sibling, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:29 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> - Remove externs and change return types in function declarations, to
> compensate change made in function definitions.
Why did you drop extern?
> - Remove space after functions and before ( in function declarations.
You should mention that this is to follow Linux kernel coding style.
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/include/rtw_cmd.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> index 299b55538788..96d71b70f476 100644
> --- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
> +++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> @@ -129,10 +129,10 @@ extern void rtw_free_cmd_obj(struct cmd_obj *pcmd);
> void rtw_stop_cmd_thread(struct adapter *adapter);
> int rtw_cmd_thread(void *context);
>
> -extern u32 rtw_init_cmd_priv (struct cmd_priv *pcmdpriv);
> +int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv);
> extern void rtw_free_cmd_priv (struct cmd_priv *pcmdpriv);
The freeing functions also have the pattern of a function that just calls
another function that has a name that starts with an _. There are also
double debug prints, one of which could go.
julia
> -extern u32 rtw_init_evt_priv (struct evt_priv *pevtpriv);
> +int rtw_init_evt_priv(struct evt_priv *pevtpriv);
> extern void rtw_free_evt_priv (struct evt_priv *pevtpriv);
> extern void rtw_evt_notify_isr(struct evt_priv *pevtpriv);
>
> --
> 2.17.1
>
> --
> 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/9e0b79cfee9a1bb0feba12741bfed92170d49af3.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type
2019-03-17 0:32 ` [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type Madhumitha Prabakaran
2019-03-17 6:29 ` [Outreachy kernel] " Julia Lawall
@ 2019-03-17 6:30 ` Julia Lawall
1 sibling, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:30 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> - Remove externs and change return types in function declarations, to
> compensate change made in function definitions.
> - Remove space after functions and before ( in function declarations.
You also should never have a patch that affects only the prototypes of
functions. Change the prototypes in the same patch where you change the
function definitions. The kernel needs to compile after every patch.
julia
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/include/rtw_cmd.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> index 299b55538788..96d71b70f476 100644
> --- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
> +++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> @@ -129,10 +129,10 @@ extern void rtw_free_cmd_obj(struct cmd_obj *pcmd);
> void rtw_stop_cmd_thread(struct adapter *adapter);
> int rtw_cmd_thread(void *context);
>
> -extern u32 rtw_init_cmd_priv (struct cmd_priv *pcmdpriv);
> +int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv);
> extern void rtw_free_cmd_priv (struct cmd_priv *pcmdpriv);
>
> -extern u32 rtw_init_evt_priv (struct evt_priv *pevtpriv);
> +int rtw_init_evt_priv(struct evt_priv *pevtpriv);
> extern void rtw_free_evt_priv (struct evt_priv *pevtpriv);
> extern void rtw_evt_notify_isr(struct evt_priv *pevtpriv);
>
> --
> 2.17.1
>
> --
> 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/9e0b79cfee9a1bb0feba12741bfed92170d49af3.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 5/7] Staging: rtl8723bs: Replace error codes with values
2019-03-17 0:32 ` [PATCH 5/7] Staging: rtl8723bs: Replace error codes with values Madhumitha Prabakaran
@ 2019-03-17 6:33 ` Julia Lawall
0 siblings, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:33 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> Replace error codes with values in order to match return types of called
> functions rtw_init_cmd_priv and rtw_init_evt_priv.
-ENOMEM is also an error code. "Use standard error codes" would be a
better description.
And this patch should be merged in with the one that changes the return
values. Admittedly it's not a build issue, but the driver would not be
working between the patch that changes the return value and this one that
fixes the call sites. You have first introduced a bug and then fixed it.
julia
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/os_dep/os_intfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> index 143e3f9b31aa..6b3def0397f0 100644
> --- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> +++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> @@ -757,7 +757,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
>
> rtw_init_hal_com_default_value(padapter);
>
> - if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == _FAIL) {
> + if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == -ENOMEM) {
> RT_TRACE(_module_os_intfs_c_, _drv_err_, ("\n Can't init cmd_priv\n"));
> ret8 = _FAIL;
> goto exit;
> @@ -765,7 +765,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
>
> padapter->cmdpriv.padapter = padapter;
>
> - if ((rtw_init_evt_priv(&padapter->evtpriv)) == _FAIL) {
> + if ((rtw_init_evt_priv(&padapter->evtpriv)) == -ENOMEM) {
> RT_TRACE(_module_os_intfs_c_, _drv_err_, ("\n Can't init evt_priv\n"));
> ret8 = _FAIL;
> goto exit;
> --
> 2.17.1
>
> --
> 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/5e5f5a18cd37b5edb56196cbe8ec6d2ce5819493.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 4/7] Staging: rtl8723bs: Remove unnecessary function declarations
2019-03-17 0:32 ` [PATCH 4/7] Staging: rtl8723bs: Remove unnecessary function declarations Madhumitha Prabakaran
@ 2019-03-17 6:34 ` Julia Lawall
0 siblings, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:34 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> Remove unnecessary function declarations, as it is never used in a
> kernel tree.
I suspect that there is no build issue here, but still it would make more
sense to make this change in the same patch where you remove the function
definition.
julia
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/include/cmd_osdep.h | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/include/cmd_osdep.h b/drivers/staging/rtl8723bs/include/cmd_osdep.h
> index 0749936df032..6940d86e93cf 100644
> --- a/drivers/staging/rtl8723bs/include/cmd_osdep.h
> +++ b/drivers/staging/rtl8723bs/include/cmd_osdep.h
> @@ -7,9 +7,6 @@
> #ifndef __CMD_OSDEP_H_
> #define __CMD_OSDEP_H_
>
> -
> -extern sint _rtw_init_cmd_priv (struct cmd_priv *pcmdpriv);
> -extern sint _rtw_init_evt_priv(struct evt_priv *pevtpriv);
> extern void _rtw_free_evt_priv (struct evt_priv *pevtpriv);
> extern void _rtw_free_cmd_priv (struct cmd_priv *pcmdpriv);
> extern sint _rtw_enqueue_cmd(struct __queue *queue, struct cmd_obj *obj);
> --
> 2.17.1
>
> --
> 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/a439f2d4d1f6a2ab47306bbcb7794379554979fc.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 3/7] Staging: rtl8723bs: Change values of error codes in functions
2019-03-17 0:32 ` [PATCH 3/7] Staging: rtl8723bs: Change values of error codes in functions Madhumitha Prabakaran
@ 2019-03-17 6:38 ` Julia Lawall
0 siblings, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:38 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> Change values of error codes _SUCCESS and _FAIL in function
> rtw_init_cmd_priv and rtw_init_evt_priv, as the code need to be
> compatible with kernel,
Technically, the code doesn't need to be compatible with the rest of the
kernel, because these specific values don't escape this driver. So
something like "to follow what is done elsewhere in the kernel".
Ultimately, these -E values should be propagated everywhere, such that
they become the failure result on some system call, but that will involve
a lot more changes, and it is better to start small.
> _SUCCESS should be 0 and _FAIL should be -ENOMEM
> (as in this case, these are memory allocating functions).
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index ab450f717f34..2890e12f6fb1 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -162,9 +162,9 @@ Caller and the rtw_cmd_thread can protect cmd_q by spin_lock.
> No irqsave is necessary.
> */
>
> -int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> +int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
There is a - and +, but I don't see the change.
julia
> {
> - int res = _SUCCESS;
> + int res = 0;
>
> init_completion(&pcmdpriv->cmd_queue_comp);
> init_completion(&pcmdpriv->terminate_cmdthread_comp);
> @@ -178,7 +178,7 @@ int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> pcmdpriv->cmd_allocated_buf = rtw_zmalloc(MAX_CMDSZ + CMDBUFF_ALIGN_SZ);
>
> if (pcmdpriv->cmd_allocated_buf == NULL) {
> - res = _FAIL;
> + res = -ENOMEM;
> goto exit;
> }
>
> @@ -187,7 +187,7 @@ int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> pcmdpriv->rsp_allocated_buf = rtw_zmalloc(MAX_RSPSZ + 4);
>
> if (pcmdpriv->rsp_allocated_buf == NULL) {
> - res = _FAIL;
> + res = -ENOMEM;
> goto exit;
> }
>
> @@ -203,7 +203,7 @@ int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> static void c2h_wk_callback(_workitem *work);
> int rtw_init_evt_priv(struct evt_priv *pevtpriv)
> {
> - int res = _SUCCESS;
> + int res = 0;
>
> /* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
> atomic_set(&pevtpriv->event_seq, 0);
> --
> 2.17.1
>
> --
> 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/df295896fcdcd016cc097c3812812582658c7319.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 2/7] Staging: rtl8723bs: Convert type of local variables and return types
2019-03-17 0:32 ` [PATCH 2/7] Staging: rtl8723bs: Convert type of local variables and return types Madhumitha Prabakaran
@ 2019-03-17 6:42 ` Julia Lawall
0 siblings, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:42 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> Convert types from sint to int for local variable 'res' and return types
> of function 'rtw_init_cmd_pri' and 'rtw_init_evt_priv', as function's
> return types are defined for error codes _SUCCESS and _FAIL.
sint is a typedef for signed int, but ints are defined by default. There
are very few occurrences of signed in in the kernel, and I don't see any
need to make the "s" explicit here.
In this patch you should change the return type in the prototype of these
functions as well.
julia
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index 793bc505fa9c..ab450f717f34 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -162,9 +162,9 @@ Caller and the rtw_cmd_thread can protect cmd_q by spin_lock.
> No irqsave is necessary.
> */
>
> -sint rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> +int rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> {
> - sint res = _SUCCESS;
> + int res = _SUCCESS;
>
> init_completion(&pcmdpriv->cmd_queue_comp);
> init_completion(&pcmdpriv->terminate_cmdthread_comp);
> @@ -201,9 +201,9 @@ sint rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> }
>
> static void c2h_wk_callback(_workitem *work);
> -sint rtw_init_evt_priv(struct evt_priv *pevtpriv)
> +int rtw_init_evt_priv(struct evt_priv *pevtpriv)
> {
> - sint res = _SUCCESS;
> + int res = _SUCCESS;
>
> /* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
> atomic_set(&pevtpriv->event_seq, 0);
> --
> 2.17.1
>
> --
> 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/6fe78b33f085802256075bd94f5e02474cefacd5.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Outreachy kernel] [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
` (6 preceding siblings ...)
2019-03-17 0:32 ` [PATCH 7/7] Staging: rtl8723bs: Add missing NULL check for kmalloc Madhumitha Prabakaran
@ 2019-03-17 6:43 ` Julia Lawall
7 siblings, 0 replies; 17+ messages in thread
From: Julia Lawall @ 2019-03-17 6:43 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> This patchset does tasks as below:
> 1) Drop wrappers and change function names
> 2) Convert type of local variables and return type
> 3) Change values of error codes in functions
> 4) Remove unnecessary function declarations
> 5) Replace error codes with values
> 6) Remove extern and change return type
> 7) Add missing NULL check for kmalloc
Even though there are some things that need to be fixed up, good job on
this so far :)
julia
>
> All these patches are relevant to an issue suggested by Coccinelle using
> ret.cocci.
>
> Madhumitha Prabakaran (7):
> Staging: rtl8723bs: Drop wrappers and change function names
> Staging: rtl8723bs: Convert type of local variables and return types
> Staging: rtl8723bs: Change values of error codes in functions
> Staging: rtl8723bs: Remove unnecessary function declarations
> Staging: rtl8723bs: Replace error codes with values
> Staging: rtl8723bs: Remove extern and change return type
> Staging: rtl8723bs: Add missing NULL check for kmalloc
>
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 30 +++++--------------
> drivers/staging/rtl8723bs/include/cmd_osdep.h | 3 --
> drivers/staging/rtl8723bs/include/rtw_cmd.h | 4 +--
> drivers/staging/rtl8723bs/os_dep/os_intfs.c | 4 +--
> 4 files changed, 12 insertions(+), 29 deletions(-)
>
> --
> 2.17.1
>
> --
> 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/cover.1552782299.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] Staging: rtl8723bs: Add missing NULL check for kmalloc
2019-03-17 0:32 ` [PATCH 7/7] Staging: rtl8723bs: Add missing NULL check for kmalloc Madhumitha Prabakaran
@ 2019-03-17 10:55 ` Greg KH
0 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2019-03-17 10:55 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: outreachy-kernel
On Sat, Mar 16, 2019 at 07:32:24PM -0500, Madhumitha Prabakaran wrote:
> Include missing NULL check for kmalloc in function rtw_init_evt_priv.
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index 2890e12f6fb1..4ea6483f4634 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -213,6 +213,8 @@ int rtw_init_evt_priv(struct evt_priv *pevtpriv)
> pevtpriv->c2h_wk_alive = false;
> pevtpriv->c2h_queue = rtw_cbuf_alloc(C2H_QUEUE_MAX_LEN+1);
>
> + if (!pevtpriv->c2h_queue)
> + return -ENOMEM;
> return res;
This is good, but if you notice, there is no need for 'res' in this
function at all, you can just return 0 for it, right?
So in the earlier patches in this series, you could just drop the local
variable completly, making this a bit simpler to do.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names
2019-03-17 0:32 ` [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names Madhumitha Prabakaran
@ 2019-03-17 10:59 ` Greg KH
0 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2019-03-17 10:59 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: outreachy-kernel
On Sat, Mar 16, 2019 at 07:32:18PM -0500, Madhumitha Prabakaran wrote:
> - Drop wrappers rtw_init_cmd_priv and rtw_init_evt_priv,
> as their only purpose is to call other functions
> _rtw_init_cmd_priv and _rtw_init_evt_priv, respectively.
> - Change function names
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 20 ++------------------
> 1 file changed, 2 insertions(+), 18 deletions(-)
I wanted to just apply this one patch, to make your patch series smaller
overall when you rebase for the next ones, but when this patch is
applied, the build breaks :(
You can never break the build at any point in a patch series. Every
patch that goes into the kernel has to be stand-alone and not cause any
problems.
So when you fix up your patch series, please do so with this in mind.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-03-17 10:59 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-17 0:32 [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
2019-03-17 0:32 ` [PATCH 1/7] Staging: rtl8723bs: Drop wrappers and change function names Madhumitha Prabakaran
2019-03-17 10:59 ` Greg KH
2019-03-17 0:32 ` [PATCH 2/7] Staging: rtl8723bs: Convert type of local variables and return types Madhumitha Prabakaran
2019-03-17 6:42 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 3/7] Staging: rtl8723bs: Change values of error codes in functions Madhumitha Prabakaran
2019-03-17 6:38 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 4/7] Staging: rtl8723bs: Remove unnecessary function declarations Madhumitha Prabakaran
2019-03-17 6:34 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 5/7] Staging: rtl8723bs: Replace error codes with values Madhumitha Prabakaran
2019-03-17 6:33 ` [Outreachy kernel] " Julia Lawall
2019-03-17 0:32 ` [PATCH 6/7] Staging: rtl8723bs: Remove extern and change return type Madhumitha Prabakaran
2019-03-17 6:29 ` [Outreachy kernel] " Julia Lawall
2019-03-17 6:30 ` Julia Lawall
2019-03-17 0:32 ` [PATCH 7/7] Staging: rtl8723bs: Add missing NULL check for kmalloc Madhumitha Prabakaran
2019-03-17 10:55 ` Greg KH
2019-03-17 6:43 ` [Outreachy kernel] [PATCH 0/7] Issue suggested by Coccinelle using ret.cocci Julia Lawall
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.