* [PATCH 0/2] Issue suggested by Coccinelle using ret.cocci
@ 2019-03-16 13:05 Madhumitha Prabakaran
2019-03-16 13:05 ` [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value Madhumitha Prabakaran
2019-03-16 13:05 ` [PATCH 2/2] Staging: rtl8723bs: Remove functions and replace return types Madhumitha Prabakaran
0 siblings, 2 replies; 6+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-16 13:05 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
This patchset does following -
1) Refactor function and check return values
2) Remove function and replace return types
Madhumitha Prabakaran (2):
Staging: rtl87223bs: Refactor function and check return value
Staging: rtl8723bs: Remove functions and replace return types
drivers/staging/rtl8723bs/core/rtw_cmd.c | 24 ++++---------------
.../staging/rtl8723bs/os_dep/osdep_service.c | 10 +++++++-
2 files changed, 13 insertions(+), 21 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value
2019-03-16 13:05 [PATCH 0/2] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
@ 2019-03-16 13:05 ` Madhumitha Prabakaran
2019-03-16 13:16 ` [Outreachy kernel] " Julia Lawall
2019-03-16 13:05 ` [PATCH 2/2] Staging: rtl8723bs: Remove functions and replace return types Madhumitha Prabakaran
1 sibling, 1 reply; 6+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-16 13:05 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
- Refactor return tenary statement in _rtw_malloc
- Check return value against NULL
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
drivers/staging/rtl8723bs/os_dep/osdep_service.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/osdep_service.c b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
index 73b87da15eb2..9dfdd13d4acd 100644
--- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
+++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
@@ -24,7 +24,15 @@ inline int RTW_STATUS_CODE(int error_code)
void *_rtw_malloc(u32 sz)
{
- return kmalloc(sz, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
+ int _sz;
+
+ if (in_interrupt())
+ _sz = kmalloc(sz, GFP_ATOMIC);
+ else
+ _sz = kmalloc(sz, GFP_KERNEL);
+ if (!_sz)
+ return -ENOMEM;
+ return _sz;
}
void *_rtw_zmalloc(u32 sz)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] Staging: rtl8723bs: Remove functions and replace return types
2019-03-16 13:05 [PATCH 0/2] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
2019-03-16 13:05 ` [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value Madhumitha Prabakaran
@ 2019-03-16 13:05 ` Madhumitha Prabakaran
2019-03-16 13:19 ` [Outreachy kernel] " Julia Lawall
1 sibling, 1 reply; 6+ messages in thread
From: Madhumitha Prabakaran @ 2019-03-16 13:05 UTC (permalink / raw)
To: gregkh, outreachy-kernel; +Cc: Madhumitha Prabakaran
- Remove functions rtw_init_cmd_priv and rtw_init_evt_priv, as its only
purpose is to call the other functions _rtw_init_cmd_priv and
_rtw_init_evt_priv, respectively.
- Rename functions from _rtw_init_cmd_priv and _rtw_init_evt_priv, to
rtw_init_cmd_priv and rtw_init_evt_priv.
- Replace return types from sint to int
- Replace local variable 'res' data type from sint to int in both the
functions.
Issue suggested by Coccinelle using ret.cocci.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
Change in v2-
- Changed the commit log
- Modified the return types from sint to int.
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 91520ca3bbad..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);
@@ -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] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value
2019-03-16 13:05 ` [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value Madhumitha Prabakaran
@ 2019-03-16 13:16 ` Julia Lawall
2019-03-16 13:21 ` Madhumthia Prabakaran
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2019-03-16 13:16 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> - Refactor return tenary statement in _rtw_malloc
> - Check return value against NULL
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
> drivers/staging/rtl8723bs/os_dep/osdep_service.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/osdep_service.c b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> index 73b87da15eb2..9dfdd13d4acd 100644
> --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> @@ -24,7 +24,15 @@ inline int RTW_STATUS_CODE(int error_code)
>
> void *_rtw_malloc(u32 sz)
> {
> - return kmalloc(sz, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
> + int _sz;
> +
> + if (in_interrupt())
> + _sz = kmalloc(sz, GFP_ATOMIC);
> + else
> + _sz = kmalloc(sz, GFP_KERNEL);
> + if (!_sz)
> + return -ENOMEM;
> + return _sz;
Sorry, but this is neither correct nor useful.
If you compile it, you should see that it is not correct.
The change that should be done is to eliminate the function entirely. The
calls should be replaced by calls to kmalloc directly, with the second
argument appropriate to the context.
But the way to do that is not by starting with this function, but rather
by working on the usage sites one by one. When there are no usage sites,
this function definition can be dropped.
julia
> }
>
> void *_rtw_zmalloc(u32 sz)
> --
> 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/79e1366235e198546f0a6781a8538ac825f775c1.1552740889.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 2/2] Staging: rtl8723bs: Remove functions and replace return types
2019-03-16 13:05 ` [PATCH 2/2] Staging: rtl8723bs: Remove functions and replace return types Madhumitha Prabakaran
@ 2019-03-16 13:19 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2019-03-16 13:19 UTC (permalink / raw)
To: Madhumitha Prabakaran; +Cc: gregkh, outreachy-kernel
On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
> - Remove functions rtw_init_cmd_priv and rtw_init_evt_priv, as its only
> purpose is to call the other functions _rtw_init_cmd_priv and
> _rtw_init_evt_priv, respectively.
> - Rename functions from _rtw_init_cmd_priv and _rtw_init_evt_priv, to
> rtw_init_cmd_priv and rtw_init_evt_priv.
> - Replace return types from sint to int
> - Replace local variable 'res' data type from sint to int in both the
> functions.
>
> Issue suggested by Coccinelle using ret.cocci.
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
>
> Change in v2-
> - Changed the commit log
> - Modified the return types from sint to int.
> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 24 ++++--------------------
> 1 file changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index 91520ca3bbad..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;
This looks ok, but a better solution would be to replace _SUCCESS by 0 and
_FAIL by -ENOMEM and fix up the call sites accordingly.
Also currently one of the functions never returns _FAIL. It should be
updated to check for failure and return -ENOMEM in this case. You should
look at how the allocated value is used, and explain why it is correct to
fail in this case.
julia
>
> 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);
> @@ -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
>
> --
> 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/9e058ab3c889d2d59b0e7c7e0296e8640a513d23.1552740889.git.madhumithabiw%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value
2019-03-16 13:16 ` [Outreachy kernel] " Julia Lawall
@ 2019-03-16 13:21 ` Madhumthia Prabakaran
0 siblings, 0 replies; 6+ messages in thread
From: Madhumthia Prabakaran @ 2019-03-16 13:21 UTC (permalink / raw)
To: Julia Lawall, outreachy-kernel
On Sat, Mar 16, 2019 at 02:16:18PM +0100, Julia Lawall wrote:
>
>
> On Sat, 16 Mar 2019, Madhumitha Prabakaran wrote:
>
> > - Refactor return tenary statement in _rtw_malloc
> > - Check return value against NULL
> >
> > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > ---
> > drivers/staging/rtl8723bs/os_dep/osdep_service.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/os_dep/osdep_service.c b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > index 73b87da15eb2..9dfdd13d4acd 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > @@ -24,7 +24,15 @@ inline int RTW_STATUS_CODE(int error_code)
> >
> > void *_rtw_malloc(u32 sz)
> > {
> > - return kmalloc(sz, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
> > + int _sz;
> > +
> > + if (in_interrupt())
> > + _sz = kmalloc(sz, GFP_ATOMIC);
> > + else
> > + _sz = kmalloc(sz, GFP_KERNEL);
> > + if (!_sz)
> > + return -ENOMEM;
> > + return _sz;
>
> Sorry, but this is neither correct nor useful.
>
> If you compile it, you should see that it is not correct.
>
> The change that should be done is to eliminate the function entirely. The
> calls should be replaced by calls to kmalloc directly, with the second
> argument appropriate to the context.
I will work on this and resend it
>
> But the way to do that is not by starting with this function, but rather
> by working on the usage sites one by one. When there are no usage sites,
> this function definition can be dropped.
>
> julia
>
> > }
> >
> > void *_rtw_zmalloc(u32 sz)
> > --
> > 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/79e1366235e198546f0a6781a8538ac825f775c1.1552740889.git.madhumithabiw%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-16 13:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-16 13:05 [PATCH 0/2] Issue suggested by Coccinelle using ret.cocci Madhumitha Prabakaran
2019-03-16 13:05 ` [PATCH 1/2] Staging: rtl87223bs: Refactor function and check return value Madhumitha Prabakaran
2019-03-16 13:16 ` [Outreachy kernel] " Julia Lawall
2019-03-16 13:21 ` Madhumthia Prabakaran
2019-03-16 13:05 ` [PATCH 2/2] Staging: rtl8723bs: Remove functions and replace return types Madhumitha Prabakaran
2019-03-16 13:19 ` [Outreachy kernel] " 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.