Re: [PATCH] staging: rtl8723bs: clean up style and dead code in rtw_recv.c

Em ter., 23 de jun. de 2026 às 18:11, Andre Moreira
<[email protected]> escreveu:
>
> Thanks for the review, Dan.
>
> I see how removing the 'else' broke the loop logic there. Thanks for the explanation and for catching that.
>
> It's great that this helped identify a pattern for a new Smatch check. I'll make sure to double-check the control flow on these legacy cleanups next time.
>
> Regards,
> André
>
> Em ter., 23 de jun. de 2026 às 02:36, Dan Carpenter <[email protected]> escreveu:
>>
>> On Mon, Jun 22, 2026 at 05:02:39PM -0300, André Moreira wrote:
>> > Fix checkpatch.pl warnings in rtw_recv.c by removing an explicit '== true'
>> > comparison with extra parentheses from check_fwstate() and dropping a
>> > redundant 'else' block after an early return.
>> >
>> > Signed-off-by: André Moreira <[email protected]>
>> > ---
>> >  drivers/staging/rtl8723bs/core/rtw_recv.c | 5 ++---
>> >  1 file changed, 2 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
>> > index f78194d508dfc..3fb48a8a52b17 100644
>> > --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
>> > +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
>> > @@ -1597,7 +1597,7 @@ static signed int wlanhdr_to_ethhdr(union recv_frame *precvframe)
>> >       eth_type = ntohs(be_tmp); /* pattrib->ether_type */
>> >       pattrib->eth_type = eth_type;
>> >
>> > -     if ((check_fwstate(pmlmepriv, WIFI_MP_STATE) == true)) {
>> > +     if (check_fwstate(pmlmepriv, WIFI_MP_STATE)) {
>>
>>
>> Someone already did this.
>>
>> >               ptr += rmv_len;
>> >               *ptr = 0x87;
>> >               *(ptr + 1) = 0x12;
>> > @@ -1841,8 +1841,7 @@ static int enqueue_reorder_recvframe(struct recv_reorder_ctrl *preorder_ctrl, un
>> >                       /* Duplicate entry is found!! Do not insert current entry. */
>> >                       /* spin_unlock_irqrestore(&ppending_recvframe_queue->lock, irql); */
>> >                       return false;
>> > -             else
>> > -                     break;
>> > +             break;
>>
>> This change introduces a bug.  I created a Smatch check based on the
>> new bug.  It's in the Smatch devel branch.
>> https://github.com/error27/smatch/blob/devel/check_pointless_loop.c
>>
>> KTODO: review loops that are really if statements
>>
>> Some of these have comments where it was originally intended to be
>> a loop but it lead to bugs so they hacked around it by only looping
>> one time.  These are mostly quite old code and they seem to be working
>> okay so probably they are false positives in one way or another but
>> just I feel like most would be more readable as an if statement.
>>
>> kernel/rcu/tasks.h:1521 rcu_tasks_verify_self_tests() warn: replace while loop with if statement?
>> kernel/bpf/btf.c:7383 btf_struct_access() warn: replace while loop with if statement?
>> drivers/net/wireguard/selftest/ratelimiter.c:170 wg_ratelimiter_selftest() warn: replace while loop with if statement?
>> drivers/video/fbdev/sis/sis_main.c:2226 sisfb_sense_crt1() warn: replace while loop with if statement?
>> drivers/video/fbdev/sis/sis_main.c:2228 sisfb_sense_crt1() warn: replace while loop with if statement?
>> drivers/video/fbdev/aty/radeon_base.c:1556 radeon_calc_pll_regs() warn: replace while loop with if statement?
>> drivers/media/pci/tw686x/tw686x-video.c:133 tw686x_memcpy_buf_refill() warn: replace while loop with if statement?
>> drivers/media/pci/tw686x/tw686x-video.c:159 tw686x_contig_buf_refill() warn: replace while loop with if statement?
>> drivers/ntb/test/ntb_perf.c:462 perf_cmd_recv() warn: replace while loop with if statement?
>> fs/afs/rxrpc.c:526 afs_deliver_to_call() warn: replace while loop with if statement?
>> fs/xfs/scrub/attr_repair.c:532 xrep_xattr_find_buf() warn: replace while loop with if statement?
>>
>> regards,
>> dan carpenter