staging: r8188eu: remove unwanted variable implementation
Commit Message
Local variables intended as the function return value are
initialized but their value does not change during function
execution. The initial value assigned to the variable is simply
returned to the caller. This makes the variable declaration
unnecessary and the initial value can be directly returned.
The patch is produced using the following coccicheck options:
COCCI=./scripts/coccinelle/misc/returnvar.cocci
M=driver/staging/r8188eu/
MODE=patch
Signed-off-by: Deepak R Varma <drv@mailo.com>
---
drivers/staging/r8188eu/core/rtw_ap.c | 5 ++---
drivers/staging/r8188eu/core/rtw_recv.c | 3 +--
drivers/staging/r8188eu/hal/rtl8188eu_xmit.c | 3 +--
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 +---
4 files changed, 5 insertions(+), 10 deletions(-)
--
2.34.1
Comments
On Tue, 1 Nov 2022, Deepak R Varma wrote:
> Local variables intended as the function return value are
> initialized but their value does not change during function
> execution. The initial value assigned to the variable is simply
> returned to the caller. This makes the variable declaration
> unnecessary and the initial value can be directly returned.
I think it would be better to make an argument for each case as to why the
change is correct, and no more interesting return value is needed. For
example, looking at the code in
drivers/staging/r8188eu/hal/rtl8188eu_xmit.c, it seems that the variable
pull was used in a more interesting way previously, but some previous
patches removed the relevant code.
julia
>
> The patch is produced using the following coccicheck options:
> COCCI=./scripts/coccinelle/misc/returnvar.cocci
> M=driver/staging/r8188eu/
> MODE=patch
>
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> drivers/staging/r8188eu/core/rtw_ap.c | 5 ++---
> drivers/staging/r8188eu/core/rtw_recv.c | 3 +--
> drivers/staging/r8188eu/hal/rtl8188eu_xmit.c | 3 +--
> drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 +---
> 4 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_ap.c b/drivers/staging/r8188eu/core/rtw_ap.c
> index 24eb8dce9bfe..9eaf345e6a00 100644
> --- a/drivers/staging/r8188eu/core/rtw_ap.c
> +++ b/drivers/staging/r8188eu/core/rtw_ap.c
> @@ -1020,7 +1020,6 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
> int rtw_sta_flush(struct adapter *padapter)
> {
> struct list_head *phead, *plist;
> - int ret = 0;
> struct sta_info *psta = NULL;
> struct sta_priv *pstapriv = &padapter->stapriv;
> struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> @@ -1028,7 +1027,7 @@ int rtw_sta_flush(struct adapter *padapter)
> u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>
> if ((pmlmeinfo->state & 0x03) != WIFI_FW_AP_STATE)
> - return ret;
> + return 0;
>
> spin_lock_bh(&pstapriv->asoc_list_lock);
> phead = &pstapriv->asoc_list;
> @@ -1051,7 +1050,7 @@ int rtw_sta_flush(struct adapter *padapter)
>
> associated_clients_update(padapter, true);
>
> - return ret;
> + return 0;
> }
>
> /* called > TSR LEVEL for USB or SDIO Interface*/
> diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> index 4b68a543f68b..94f85cd7038d 100644
> --- a/drivers/staging/r8188eu/core/rtw_recv.c
> +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> @@ -1415,7 +1415,6 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
>
> struct recv_priv *precvpriv = &padapter->recvpriv;
> struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue;
> - int ret = _SUCCESS;
>
> nr_subframes = 0;
>
> @@ -1513,7 +1512,7 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
> prframe->len = 0;
> rtw_free_recvframe(prframe, pfree_recv_queue);/* free this recv_frame */
>
> - return ret;
> + return _SUCCESS;
> }
>
> static bool check_indicate_seq(struct recv_reorder_ctrl *preorder_ctrl, u16 seq_num)
> diff --git a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> index 8e4a5acc0b18..6d1f56d1f9d7 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> @@ -149,7 +149,6 @@ static void fill_txdesc_phy(struct pkt_attrib *pattrib, __le32 *pdw)
>
> static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bagg_pkt)
> {
> - int pull = 0;
> uint qsel;
> u8 data_rate, pwr_status, offset;
> struct adapter *adapt = pxmitframe->padapter;
> @@ -295,7 +294,7 @@ static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bag
> ODM_SetTxAntByTxInfo_88E(&haldata->odmpriv, pmem, pattrib->mac_id);
>
> rtl8188eu_cal_txdesc_chksum(ptxdesc);
> - return pull;
> + return 0;
> }
>
> /* for non-agg data frame or management frame */
> diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> index 8516e253bb03..59a97c5fb80c 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -2979,8 +2979,6 @@ static int rtw_p2p_set(struct net_device *dev,
> struct iw_request_info *info,
> union iwreq_data *wrqu, char *extra)
> {
> - int ret = 0;
> -
> if (!memcmp(extra, "enable =", 7)) {
> rtw_wext_p2p_enable(dev, info, wrqu, &extra[7]);
> } else if (!memcmp(extra, "setDN =", 6)) {
> @@ -3027,7 +3025,7 @@ static int rtw_p2p_set(struct net_device *dev,
> rtw_p2p_set_persistent(dev, info, wrqu, &extra[11]);
> }
>
> - return ret;
> + return 0;
> }
>
> static int rtw_p2p_get2(struct net_device *dev,
> --
> 2.34.1
>
>
>
>
>
On Tue, Nov 01, 2022 at 06:14:01PM +0100, Julia Lawall wrote:
>
>
> On Tue, 1 Nov 2022, Deepak R Varma wrote:
>
> > Local variables intended as the function return value are
> > initialized but their value does not change during function
> > execution. The initial value assigned to the variable is simply
> > returned to the caller. This makes the variable declaration
> > unnecessary and the initial value can be directly returned.
>
> I think it would be better to make an argument for each case as to why the
> change is correct, and no more interesting return value is needed. For
> example, looking at the code in
> drivers/staging/r8188eu/hal/rtl8188eu_xmit.c, it seems that the variable
> pull was used in a more interesting way previously, but some previous
> patches removed the relevant code.
Thank you Julia for the hints. I learned that git blame will not show commits
associated with deleted/replaced lines. Used pickaxe tool to identify the old
commits. It was good learning. Thank you. v2 is submitted for review and feedback.
>
> julia
>
> >
> > The patch is produced using the following coccicheck options:
> > COCCI=./scripts/coccinelle/misc/returnvar.cocci
> > M=driver/staging/r8188eu/
> > MODE=patch
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> > drivers/staging/r8188eu/core/rtw_ap.c | 5 ++---
> > drivers/staging/r8188eu/core/rtw_recv.c | 3 +--
> > drivers/staging/r8188eu/hal/rtl8188eu_xmit.c | 3 +--
> > drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 +---
> > 4 files changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_ap.c b/drivers/staging/r8188eu/core/rtw_ap.c
> > index 24eb8dce9bfe..9eaf345e6a00 100644
> > --- a/drivers/staging/r8188eu/core/rtw_ap.c
> > +++ b/drivers/staging/r8188eu/core/rtw_ap.c
> > @@ -1020,7 +1020,6 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
> > int rtw_sta_flush(struct adapter *padapter)
> > {
> > struct list_head *phead, *plist;
> > - int ret = 0;
> > struct sta_info *psta = NULL;
> > struct sta_priv *pstapriv = &padapter->stapriv;
> > struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> > @@ -1028,7 +1027,7 @@ int rtw_sta_flush(struct adapter *padapter)
> > u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> >
> > if ((pmlmeinfo->state & 0x03) != WIFI_FW_AP_STATE)
> > - return ret;
> > + return 0;
> >
> > spin_lock_bh(&pstapriv->asoc_list_lock);
> > phead = &pstapriv->asoc_list;
> > @@ -1051,7 +1050,7 @@ int rtw_sta_flush(struct adapter *padapter)
> >
> > associated_clients_update(padapter, true);
> >
> > - return ret;
> > + return 0;
> > }
> >
> > /* called > TSR LEVEL for USB or SDIO Interface*/
> > diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> > index 4b68a543f68b..94f85cd7038d 100644
> > --- a/drivers/staging/r8188eu/core/rtw_recv.c
> > +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> > @@ -1415,7 +1415,6 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
> >
> > struct recv_priv *precvpriv = &padapter->recvpriv;
> > struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue;
> > - int ret = _SUCCESS;
> >
> > nr_subframes = 0;
> >
> > @@ -1513,7 +1512,7 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
> > prframe->len = 0;
> > rtw_free_recvframe(prframe, pfree_recv_queue);/* free this recv_frame */
> >
> > - return ret;
> > + return _SUCCESS;
> > }
> >
> > static bool check_indicate_seq(struct recv_reorder_ctrl *preorder_ctrl, u16 seq_num)
> > diff --git a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> > index 8e4a5acc0b18..6d1f56d1f9d7 100644
> > --- a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> > +++ b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> > @@ -149,7 +149,6 @@ static void fill_txdesc_phy(struct pkt_attrib *pattrib, __le32 *pdw)
> >
> > static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bagg_pkt)
> > {
> > - int pull = 0;
> > uint qsel;
> > u8 data_rate, pwr_status, offset;
> > struct adapter *adapt = pxmitframe->padapter;
> > @@ -295,7 +294,7 @@ static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bag
> > ODM_SetTxAntByTxInfo_88E(&haldata->odmpriv, pmem, pattrib->mac_id);
> >
> > rtl8188eu_cal_txdesc_chksum(ptxdesc);
> > - return pull;
> > + return 0;
> > }
> >
> > /* for non-agg data frame or management frame */
> > diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > index 8516e253bb03..59a97c5fb80c 100644
> > --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > @@ -2979,8 +2979,6 @@ static int rtw_p2p_set(struct net_device *dev,
> > struct iw_request_info *info,
> > union iwreq_data *wrqu, char *extra)
> > {
> > - int ret = 0;
> > -
> > if (!memcmp(extra, "enable =", 7)) {
> > rtw_wext_p2p_enable(dev, info, wrqu, &extra[7]);
> > } else if (!memcmp(extra, "setDN =", 6)) {
> > @@ -3027,7 +3025,7 @@ static int rtw_p2p_set(struct net_device *dev,
> > rtw_p2p_set_persistent(dev, info, wrqu, &extra[11]);
> > }
> >
> > - return ret;
> > + return 0;
> > }
> >
> > static int rtw_p2p_get2(struct net_device *dev,
> > --
> > 2.34.1
> >
> >
> >
> >
> >
>
On Wed, 2 Nov 2022, Deepak R Varma wrote:
> On Tue, Nov 01, 2022 at 06:14:01PM +0100, Julia Lawall wrote:
> >
> >
> > On Tue, 1 Nov 2022, Deepak R Varma wrote:
> >
> > > Local variables intended as the function return value are
> > > initialized but their value does not change during function
> > > execution. The initial value assigned to the variable is simply
> > > returned to the caller. This makes the variable declaration
> > > unnecessary and the initial value can be directly returned.
> >
> > I think it would be better to make an argument for each case as to why the
> > change is correct, and no more interesting return value is needed. For
> > example, looking at the code in
> > drivers/staging/r8188eu/hal/rtl8188eu_xmit.c, it seems that the variable
> > pull was used in a more interesting way previously, but some previous
> > patches removed the relevant code.
>
> Thank you Julia for the hints. I learned that git blame will not show commits
> associated with deleted/replaced lines. Used pickaxe tool to identify the old
> commits. It was good learning. Thank you. v2 is submitted for review and feedback.
OK, I just did git log -p and looked through the commits.
You can also use the argument eg -G pull to find commits that change the
number of occurrences of pull.
julia
>
> >
> > julia
> >
> > >
> > > The patch is produced using the following coccicheck options:
> > > COCCI=./scripts/coccinelle/misc/returnvar.cocci
> > > M=driver/staging/r8188eu/
> > > MODE=patch
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > > drivers/staging/r8188eu/core/rtw_ap.c | 5 ++---
> > > drivers/staging/r8188eu/core/rtw_recv.c | 3 +--
> > > drivers/staging/r8188eu/hal/rtl8188eu_xmit.c | 3 +--
> > > drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 +---
> > > 4 files changed, 5 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/staging/r8188eu/core/rtw_ap.c b/drivers/staging/r8188eu/core/rtw_ap.c
> > > index 24eb8dce9bfe..9eaf345e6a00 100644
> > > --- a/drivers/staging/r8188eu/core/rtw_ap.c
> > > +++ b/drivers/staging/r8188eu/core/rtw_ap.c
> > > @@ -1020,7 +1020,6 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
> > > int rtw_sta_flush(struct adapter *padapter)
> > > {
> > > struct list_head *phead, *plist;
> > > - int ret = 0;
> > > struct sta_info *psta = NULL;
> > > struct sta_priv *pstapriv = &padapter->stapriv;
> > > struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> > > @@ -1028,7 +1027,7 @@ int rtw_sta_flush(struct adapter *padapter)
> > > u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> > >
> > > if ((pmlmeinfo->state & 0x03) != WIFI_FW_AP_STATE)
> > > - return ret;
> > > + return 0;
> > >
> > > spin_lock_bh(&pstapriv->asoc_list_lock);
> > > phead = &pstapriv->asoc_list;
> > > @@ -1051,7 +1050,7 @@ int rtw_sta_flush(struct adapter *padapter)
> > >
> > > associated_clients_update(padapter, true);
> > >
> > > - return ret;
> > > + return 0;
> > > }
> > >
> > > /* called > TSR LEVEL for USB or SDIO Interface*/
> > > diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> > > index 4b68a543f68b..94f85cd7038d 100644
> > > --- a/drivers/staging/r8188eu/core/rtw_recv.c
> > > +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> > > @@ -1415,7 +1415,6 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
> > >
> > > struct recv_priv *precvpriv = &padapter->recvpriv;
> > > struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue;
> > > - int ret = _SUCCESS;
> > >
> > > nr_subframes = 0;
> > >
> > > @@ -1513,7 +1512,7 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
> > > prframe->len = 0;
> > > rtw_free_recvframe(prframe, pfree_recv_queue);/* free this recv_frame */
> > >
> > > - return ret;
> > > + return _SUCCESS;
> > > }
> > >
> > > static bool check_indicate_seq(struct recv_reorder_ctrl *preorder_ctrl, u16 seq_num)
> > > diff --git a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> > > index 8e4a5acc0b18..6d1f56d1f9d7 100644
> > > --- a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> > > +++ b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
> > > @@ -149,7 +149,6 @@ static void fill_txdesc_phy(struct pkt_attrib *pattrib, __le32 *pdw)
> > >
> > > static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bagg_pkt)
> > > {
> > > - int pull = 0;
> > > uint qsel;
> > > u8 data_rate, pwr_status, offset;
> > > struct adapter *adapt = pxmitframe->padapter;
> > > @@ -295,7 +294,7 @@ static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bag
> > > ODM_SetTxAntByTxInfo_88E(&haldata->odmpriv, pmem, pattrib->mac_id);
> > >
> > > rtl8188eu_cal_txdesc_chksum(ptxdesc);
> > > - return pull;
> > > + return 0;
> > > }
> > >
> > > /* for non-agg data frame or management frame */
> > > diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > > index 8516e253bb03..59a97c5fb80c 100644
> > > --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > > +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > > @@ -2979,8 +2979,6 @@ static int rtw_p2p_set(struct net_device *dev,
> > > struct iw_request_info *info,
> > > union iwreq_data *wrqu, char *extra)
> > > {
> > > - int ret = 0;
> > > -
> > > if (!memcmp(extra, "enable =", 7)) {
> > > rtw_wext_p2p_enable(dev, info, wrqu, &extra[7]);
> > > } else if (!memcmp(extra, "setDN =", 6)) {
> > > @@ -3027,7 +3025,7 @@ static int rtw_p2p_set(struct net_device *dev,
> > > rtw_p2p_set_persistent(dev, info, wrqu, &extra[11]);
> > > }
> > >
> > > - return ret;
> > > + return 0;
> > > }
> > >
> > > static int rtw_p2p_get2(struct net_device *dev,
> > > --
> > > 2.34.1
> > >
> > >
> > >
> > >
> > >
> >
>
>
>
@@ -1020,7 +1020,6 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
int rtw_sta_flush(struct adapter *padapter)
{
struct list_head *phead, *plist;
- int ret = 0;
struct sta_info *psta = NULL;
struct sta_priv *pstapriv = &padapter->stapriv;
struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
@@ -1028,7 +1027,7 @@ int rtw_sta_flush(struct adapter *padapter)
u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
if ((pmlmeinfo->state & 0x03) != WIFI_FW_AP_STATE)
- return ret;
+ return 0;
spin_lock_bh(&pstapriv->asoc_list_lock);
phead = &pstapriv->asoc_list;
@@ -1051,7 +1050,7 @@ int rtw_sta_flush(struct adapter *padapter)
associated_clients_update(padapter, true);
- return ret;
+ return 0;
}
/* called > TSR LEVEL for USB or SDIO Interface*/
@@ -1415,7 +1415,6 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
struct recv_priv *precvpriv = &padapter->recvpriv;
struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue;
- int ret = _SUCCESS;
nr_subframes = 0;
@@ -1513,7 +1512,7 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
prframe->len = 0;
rtw_free_recvframe(prframe, pfree_recv_queue);/* free this recv_frame */
- return ret;
+ return _SUCCESS;
}
static bool check_indicate_seq(struct recv_reorder_ctrl *preorder_ctrl, u16 seq_num)
@@ -149,7 +149,6 @@ static void fill_txdesc_phy(struct pkt_attrib *pattrib, __le32 *pdw)
static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bagg_pkt)
{
- int pull = 0;
uint qsel;
u8 data_rate, pwr_status, offset;
struct adapter *adapt = pxmitframe->padapter;
@@ -295,7 +294,7 @@ static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bag
ODM_SetTxAntByTxInfo_88E(&haldata->odmpriv, pmem, pattrib->mac_id);
rtl8188eu_cal_txdesc_chksum(ptxdesc);
- return pull;
+ return 0;
}
/* for non-agg data frame or management frame */
@@ -2979,8 +2979,6 @@ static int rtw_p2p_set(struct net_device *dev,
struct iw_request_info *info,
union iwreq_data *wrqu, char *extra)
{
- int ret = 0;
-
if (!memcmp(extra, "enable =", 7)) {
rtw_wext_p2p_enable(dev, info, wrqu, &extra[7]);
} else if (!memcmp(extra, "setDN =", 6)) {
@@ -3027,7 +3025,7 @@ static int rtw_p2p_set(struct net_device *dev,
rtw_p2p_set_persistent(dev, info, wrqu, &extra[11]);
}
- return ret;
+ return 0;
}
static int rtw_p2p_get2(struct net_device *dev,