Re: [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c

[Pkshih]

On Tue, 2018-08-07 at 09:23 -0500, Larry Finger wrote:
> On 08/06/2018 04:42 PM, valdis.kletnieks@xxxxxx wrote:
> > On Mon, 06 Aug 2018 12:54:40 +0800, YueHaibing said:
> >> Fix following coccinelle warning:
> >>
> >> ./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2952:2-4: WARNING: possible
> condition with no effect (if == else)
> >Â
> >>ÂÂÂ	/* sw mechanism */
> >>ÂÂÂ	if (BTC_WIFI_BW_HT40 == wifi_bw) {
> >> -		if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
> >> -		ÂÂÂÂ(wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
> >> -			btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> -						ÂÂfalse, false);
> >> -		} else {
> >> -			btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> -						ÂÂfalse, false);
> >> -		}
> >> +		btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> +					ÂÂfalse, false);
> >>ÂÂÂ	} else {
> >Â
> > Rather than blindly fixing this, perhaps a bit of thought needs to be
> > applied to why this code looks like this in the first place.
> >Â
> > See commit c6821613e653aÂÂ(which looks like the bletcherous "do too many
> > things at once" commit indeed), although the actual diff appears to be a
> > "no harm, no foul" against this commit, where the issue already existed.
> >Â
> > commit aa45a673b291fd761275493bc15316d79555ed55
> > Author: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> > Date:ÂÂÂFri Feb 28 15:16:43 2014 -0600
> >Â
> >ÂÂÂÂÂÂrtlwifi: btcoexist: Add new mini driver
> >Â
> > Larry? Can you reach back to 2014 and remember why this code
> > looked like this in the first place?
> 
> The base code came from Realtek. My only part in getting it into the kernel wasÂ
> to clean up the checkpatch and Sparse errors and warnings, and submit it. I wasÂ
> a "script kiddy" just like the authors of the current patches. The onlyÂ
> difference is that I was getting drivers into the kernel so that users hardwareÂ
> would work.
> 
> Any record of whether these duplicate branches are the result of incorrect copyÂ
> and paste, or just extraneous code, would be at Realtek in their version controlÂ
> history. I have never had access to such archives, nor have I ever had anÂ
> non-disclosure agreement with Realtek.
> 
> Ping-Ke Shih, who is Cc'd on these messages, might be able to answer theseÂ
> questions.

These branches is used to improve user experience according to RSSI strength, but
it has not significant improvement so the same arguments become duplicate branches.
I check the latest code ofÂhalbtc8723b2ant.c, there are more than one statements
within if-else branch to improve performance, but the statements mentioned byÂ
this patch are still the same. So, these duplicate branches can be safely removed.

> 
> For the moment, these simplifications could be applied as long as they areÂ
> correctly done. After all, they are not changing what is already there and thatÂ
> stops any other person that knows how to run coccinelle from submitting the sameÂ
> patch in the future. Why "kick the can down the road"? If PK can find that thereÂ
> was an error in the original code, he can submit a "Fixes" patch.
> 
>