Optimize variable initialization by integrating the initialization
directly into the variable declaration in cases where the initialization
is simple and doesn't depend on other variables or complex expressions.
This makes the code more concise and readable.
Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
.../staging/rtl8723bs/hal/rtl8723bs_xmit.c | 56 ++++++-------------
1 file changed, 16 insertions(+), 40 deletions(-)
diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
index 5dc1c12fe03e..ebe9562a9606 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
@@ -120,13 +120,10 @@ static s32 rtl8723_dequeue_writeport(struct adapter *padapter)
*/
s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
{
- struct xmit_priv *pxmitpriv;
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
u8 queue_empty, queue_pending;
s32 ret;
-
- pxmitpriv = &padapter->xmitpriv;
-
if (wait_for_completion_interruptible(&pxmitpriv->xmit_comp)) {
netdev_emerg(padapter->pnetdev,
"%s: down SdioXmitBufSema fail!\n", __func__);
@@ -168,10 +165,10 @@ s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
*/
static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv)
{
- s32 err, ret;
+ s32 err = 0, ret;
u32 k = 0;
- struct hw_xmit *hwxmits, *phwxmit;
- u8 idx, hwentry;
+ struct hw_xmit *hwxmits = pxmitpriv->hwxmits, *phwxmit;
+ u8 idx, hwentry = pxmitpriv->hwxmit_entry;
struct tx_servq *ptxservq;
struct list_head *sta_plist, *sta_phead, *frame_plist, *frame_phead, *tmp;
struct xmit_frame *pxmitframe;
@@ -181,9 +178,6 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
u8 txdesc_size = TXDESC_SIZE;
int inx[4];
- err = 0;
- hwxmits = pxmitpriv->hwxmits;
- hwentry = pxmitpriv->hwxmit_entry;
ptxservq = NULL;
pxmitframe = NULL;
pframe_queue = NULL;
@@ -326,8 +320,7 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
/* dump xmit_buf to hw tx fifo */
if (pxmitbuf) {
if (pxmitbuf->len > 0) {
- struct xmit_frame *pframe;
- pframe = (struct xmit_frame *)pxmitbuf->priv_data;
+ struct xmit_frame *pframe = (struct xmit_frame *)pxmitbuf->priv_data;
pframe->agg_num = k;
pxmitbuf->agg_num = k;
rtl8723b_update_txdesc(pframe, pframe->buf_addr);
@@ -357,12 +350,9 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
*/
static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
{
- struct xmit_priv *pxmitpriv;
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
s32 ret;
-
- pxmitpriv = &padapter->xmitpriv;
-
if (wait_for_completion_interruptible(&pxmitpriv->SdioXmitStart)) {
netdev_emerg(padapter->pnetdev, "%s: SdioXmitStart fail!\n",
__func__);
@@ -408,13 +398,9 @@ static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
int rtl8723bs_xmit_thread(void *context)
{
- s32 ret;
- struct adapter *padapter;
- struct xmit_priv *pxmitpriv;
-
- ret = _SUCCESS;
- padapter = context;
- pxmitpriv = &padapter->xmitpriv;
+ s32 ret = _SUCCESS;
+ struct adapter *padapter = context;
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
allow_signal(SIGTERM);
@@ -435,16 +421,13 @@ s32 rtl8723bs_mgnt_xmit(
)
{
s32 ret = _SUCCESS;
- struct pkt_attrib *pattrib;
- struct xmit_buf *pxmitbuf;
+ struct pkt_attrib *pattrib = &pmgntframe->attrib;
+ struct xmit_buf *pxmitbuf = pmgntframe->pxmitbuf;
struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
u8 *pframe = (u8 *)(pmgntframe->buf_addr) + TXDESC_OFFSET;
u8 txdesc_size = TXDESC_SIZE;
- pattrib = &pmgntframe->attrib;
- pxmitbuf = pmgntframe->pxmitbuf;
-
rtl8723b_update_txdesc(pmgntframe, pmgntframe->buf_addr);
pxmitbuf->len = txdesc_size + pattrib->last_txcmdsz;
@@ -519,9 +502,8 @@ s32 rtl8723bs_hal_xmitframe_enqueue(
)
{
struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
- s32 err;
+ s32 err = rtw_xmitframe_enqueue(padapter, pxmitframe);
- err = rtw_xmitframe_enqueue(padapter, pxmitframe);
if (err != _SUCCESS) {
rtw_free_xmitframe(pxmitpriv, pxmitframe);
@@ -543,10 +525,7 @@ s32 rtl8723bs_hal_xmitframe_enqueue(
s32 rtl8723bs_init_xmit_priv(struct adapter *padapter)
{
struct xmit_priv *xmitpriv = &padapter->xmitpriv;
- struct hal_com_data *phal;
-
-
- phal = GET_HAL_DATA(padapter);
+ struct hal_com_data *phal = GET_HAL_DATA(padapter);
spin_lock_init(&phal->SdioTxFIFOFreePageLock);
init_completion(&xmitpriv->SdioXmitStart);
@@ -557,16 +536,13 @@ s32 rtl8723bs_init_xmit_priv(struct adapter *padapter)
void rtl8723bs_free_xmit_priv(struct adapter *padapter)
{
- struct xmit_priv *pxmitpriv;
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
struct xmit_buf *pxmitbuf;
- struct __queue *pqueue;
- struct list_head *plist, *phead;
+ struct __queue *pqueue = &pxmitpriv->pending_xmitbuf_queue;
+ struct list_head *plist, *phead = get_list_head(pqueue);
struct list_head tmplist;
- pxmitpriv = &padapter->xmitpriv;
- pqueue = &pxmitpriv->pending_xmitbuf_queue;
- phead = get_list_head(pqueue);
INIT_LIST_HEAD(&tmplist);
spin_lock_bh(&pqueue->lock);
--
2.43.0On Sat, Apr 05, 2025 at 06:14:49AM +0300, Erick Karanja wrote:
> Optimize variable initialization by integrating the initialization
> directly into the variable declaration in cases where the initialization
> is simple and doesn't depend on other variables or complex expressions.
> This makes the code more concise and readable.
>
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
> .../staging/rtl8723bs/hal/rtl8723bs_xmit.c | 56 ++++++-------------
> 1 file changed, 16 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> index 5dc1c12fe03e..ebe9562a9606 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> @@ -120,13 +120,10 @@ static s32 rtl8723_dequeue_writeport(struct adapter *padapter)
> */
> s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
> {
> - struct xmit_priv *pxmitpriv;
> + struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> u8 queue_empty, queue_pending;
> s32 ret;
>
> -
> - pxmitpriv = &padapter->xmitpriv;
> -
> if (wait_for_completion_interruptible(&pxmitpriv->xmit_comp)) {
> netdev_emerg(padapter->pnetdev,
> "%s: down SdioXmitBufSema fail!\n", __func__);
> @@ -168,10 +165,10 @@ s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
> */
> static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv)
> {
> - s32 err, ret;
> + s32 err = 0, ret;
> u32 k = 0;
> - struct hw_xmit *hwxmits, *phwxmit;
> - u8 idx, hwentry;
> + struct hw_xmit *hwxmits = pxmitpriv->hwxmits, *phwxmit;
> + u8 idx, hwentry = pxmitpriv->hwxmit_entry;
These lines are NOT more understandable and readable at all, sorry. You
are mixing pre-initialized variables with not-initialized ones, making
this harder to read and maintain over time.
Which would you want to come back to in 5 years to try to understand
what is going on?
Keep it simple please.
thanks,
greg k-h
On Sat, 5 Apr 2025, Greg KH wrote:
> On Sat, Apr 05, 2025 at 06:14:49AM +0300, Erick Karanja wrote:
> > Optimize variable initialization by integrating the initialization
> > directly into the variable declaration in cases where the initialization
> > is simple and doesn't depend on other variables or complex expressions.
> > This makes the code more concise and readable.
> >
> > Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> > ---
> > .../staging/rtl8723bs/hal/rtl8723bs_xmit.c | 56 ++++++-------------
> > 1 file changed, 16 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > index 5dc1c12fe03e..ebe9562a9606 100644
> > --- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > +++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > @@ -120,13 +120,10 @@ static s32 rtl8723_dequeue_writeport(struct adapter *padapter)
> > */
> > s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
> > {
> > - struct xmit_priv *pxmitpriv;
> > + struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> > u8 queue_empty, queue_pending;
> > s32 ret;
> >
> > -
> > - pxmitpriv = &padapter->xmitpriv;
> > -
> > if (wait_for_completion_interruptible(&pxmitpriv->xmit_comp)) {
> > netdev_emerg(padapter->pnetdev,
> > "%s: down SdioXmitBufSema fail!\n", __func__);
> > @@ -168,10 +165,10 @@ s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
> > */
> > static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv)
> > {
> > - s32 err, ret;
> > + s32 err = 0, ret;
> > u32 k = 0;
> > - struct hw_xmit *hwxmits, *phwxmit;
> > - u8 idx, hwentry;
> > + struct hw_xmit *hwxmits = pxmitpriv->hwxmits, *phwxmit;
> > + u8 idx, hwentry = pxmitpriv->hwxmit_entry;
>
> These lines are NOT more understandable and readable at all, sorry. You
> are mixing pre-initialized variables with not-initialized ones, making
> this harder to read and maintain over time.
>
> Which would you want to come back to in 5 years to try to understand
> what is going on?
>
> Keep it simple please.
This is another deficiency of Coccinelle. It doesn't really see the case
where there are multiple declarations on a single line, since it is
generally more valuable to be able to work on them individually.
I guess the first change above would be ok though.
julia
On Sat, Apr 05, 2025 at 04:41:37AM -0400, Julia Lawall wrote: > > I guess the first change above would be ok though. Here is the first change: - struct xmit_priv *pxmitpriv; + struct xmit_priv *pxmitpriv = &padapter->xmitpriv; u8 queue_empty, queue_pending; s32 ret; - - pxmitpriv = &padapter->xmitpriv; - Yes. This is a nice change. regards, dan carpenter
© 2016 - 2026 Red Hat, Inc.