Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: Change return type and type of local variable

[Madhumthia Prabakaran <madhumithabiw@gmail.com>]

On Wed, Mar 20, 2019 at 07:26:21AM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 19 Mar 2019, Madhumitha Prabakaran wrote:
> 
> > Change type of local variable and return type of function rtw_startbss_cmd,
> > as function's return type are defined for standard error codes. Change
> > corresponding function declarations and call sites. Along with
> > modifications, remove extern from the function declaration to maintain
> > Linux kernel coding style.
> 
> The changes look good, but the log message isn't really ideal to
> understand what is going on.
> 
> You could start by stating what happens:
> 
> rtw_startbss_cmd can return the result of calling rtw_enqueue_cmd, which
> can return the result of calling _rtw_enqueue_cmd, but they all have
> different return types (u8, u32, and sint, respectively).
> 
> As all of these functions return error codes, change all the return types
> to int, in preparation for converting these functions to use standard
> kernel error codes.

This makes more sense, I will edit it in my commit log.

> 
> In checking these changes, I also saw nearby the function rtw_cmd_filter.
> Currently that returns int and _SUCCESS and _FAIL.  Given the name, it
> might make more sense for this function to return true in the _SUCCESS
> case and false in the _FAIL case, and thus to have a bool return type.
> The function also has a local variable called bAllow of type u8 that would
> be better off with the name allow and type bool.  There is also a strange
> prototype of the function that is right above the definition of the
> function itself.  This is not needed.

Can I submit this change in another patch? Combining above change and
this one as a patch series.


> 
> julia
> 
> 
> > Issue suggested by Coccinelle.
> >
> > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > ---
> >  drivers/staging/rtl8723bs/core/rtw_cmd.c      | 8 ++++----
> >  drivers/staging/rtl8723bs/include/cmd_osdep.h | 2 +-
> >  drivers/staging/rtl8723bs/include/rtw_cmd.h   | 4 ++--
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > index bd3a5a23a418..04c6927dab8b 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > @@ -256,7 +256,7 @@ ISR/Call-Back functions can't call this sub-function.
> >
> >  */
> >
> > -sint	_rtw_enqueue_cmd(struct __queue *queue, struct cmd_obj *obj)
> > +int _rtw_enqueue_cmd(struct __queue *queue, struct cmd_obj *obj)
> >  {
> >  	_irqL irqL;
> >
> > @@ -331,7 +331,7 @@ int rtw_cmd_filter(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
> >
> >
> >
> > -u32 rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
> > +int rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
> >  {
> >  	int res = _FAIL;
> >  	struct adapter *padapter = pcmdpriv->padapter;
> > @@ -719,12 +719,12 @@ u8 rtw_createbss_cmd(struct adapter  *padapter)
> >  	return res;
> >  }
> >
> > -u8 rtw_startbss_cmd(struct adapter  *padapter, int flags)
> > +int rtw_startbss_cmd(struct adapter  *padapter, int flags)
> >  {
> >  	struct cmd_obj *pcmd;
> >  	struct cmd_priv  *pcmdpriv = &padapter->cmdpriv;
> >  	struct submit_ctx sctx;
> > -	u8 res = _SUCCESS;
> > +	int res = _SUCCESS;
> >
> >  	if (flags & RTW_CMDF_DIRECTLY) {
> >  		/* no need to enqueue, do the cmd hdl directly and free cmd parameter */
> > diff --git a/drivers/staging/rtl8723bs/include/cmd_osdep.h b/drivers/staging/rtl8723bs/include/cmd_osdep.h
> > index 06ab48263757..d3af9f44ad59 100644
> > --- a/drivers/staging/rtl8723bs/include/cmd_osdep.h
> > +++ b/drivers/staging/rtl8723bs/include/cmd_osdep.h
> > @@ -12,7 +12,7 @@ int rtw_init_cmd_priv(struct	cmd_priv *pcmdpriv);
> >  int 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);
> > +int _rtw_enqueue_cmd(struct __queue *queue, struct cmd_obj *obj);
> >  extern struct	cmd_obj	*_rtw_dequeue_cmd(struct __queue *queue);
> >
> >  #endif
> > diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> > index fa4ebc3d7539..b83824ca2e31 100644
> > --- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
> > +++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> > @@ -122,7 +122,7 @@ struct P2P_PS_CTWPeriod_t {
> >  	u8 CTWPeriod;	/* TU */
> >  };
> >
> > -extern u32 rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct cmd_obj *obj);
> > +int rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct cmd_obj *obj);
> >  extern struct cmd_obj *rtw_dequeue_cmd(struct cmd_priv *pcmdpriv);
> >  extern void rtw_free_cmd_obj(struct cmd_obj *pcmd);
> >
> > @@ -827,7 +827,7 @@ struct RunInThread_param
> >
> >  u8 rtw_sitesurvey_cmd(struct adapter  *padapter, struct ndis_802_11_ssid *ssid, int ssid_num, struct rtw_ieee80211_channel *ch, int ch_num);
> >  extern u8 rtw_createbss_cmd(struct adapter  *padapter);
> > -u8 rtw_startbss_cmd(struct adapter  *padapter, int flags);
> > +int rtw_startbss_cmd(struct adapter  *padapter, int flags);
> >
> >  struct sta_info;
> >  extern u8 rtw_setstakey_cmd(struct adapter  *padapter, struct sta_info *sta, u8 unicast_key, bool enqueue);
> > --
> > 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/20190320000125.8930-1-madhumithabiw%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >