Don't worry then, the explanation for humans is more than enough. On Wed, May 5, 2021 at 4:33 PM Pkshih <pkshih@xxxxxxxxxxx> wrote: > > On Wed, 2021-05-05 at 16:20 +0200, Inigo Huguet wrote: > > If it's no problem to add them, a comment STARTING WITH > > `coverity[identical_branches]` should suppress the warnings. > > > > Example: > > > > /* Explanation why this code is fine > > * and great > > */ > > /* coverity[identical_branches] */ > > if (...) > > ... > > > > Thanks! > > > > On Wed, May 5, 2021 at 4:03 PM Pkshih <pkshih@xxxxxxxxxxx> wrote: > > > > > > On Wed, 2021-05-05 at 13:01 +0000, Inigo Huguet wrote: > > > > Hi, > > > > > > > > Thanks for the info. Maybe we should consider adding some comments to > > > > clarify this? Other people might also think these are bugs... > > > > > > > > Regards, > > > > > > > > On Wed, May 5, 2021 at 2:13 PM Pkshih <pkshih@xxxxxxxxxxx> wrote: > > > > > > > > > > On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote: > > > > > > On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <ihuguet@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > Executing some static analysis on the kernel, we've got this results > > > > > > > affecting rtlwifi drivers: > > > > > > > > > > > > > > Error: IDENTICAL_BRANCHES (CWE-398): [#def212] > > > > > > > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2813: > > > > > > > identical_branches: The same code is executed regardless of whether > > > > > > > "bt_rssi_state == BTC_RSSI_STATE_HIGH || bt_rssi_state == > > > > > > > BTC_RSSI_STATE_STAY_HIGH" is true, because the 'then' and 'else' > > > > > > > branches are identical. Should one of the branches be modified, or the > > > > > > > entire 'if' statement replaced? > > > > > > > # 2811| } > > > > > > > # 2812| > > > > > > > # 2813|-> if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) || > > > > > > > # 2814| (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) { > > > > > > > # 2815| btc8821a2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 23); > > > > > > > > > > > > > > Error: IDENTICAL_BRANCHES (CWE-398): [#def213] > > > > > > > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2947: > > > > > > > identical_branches: The same code is executed regardless of whether > > > > > > > "bt_rssi_state == BTC_RSSI_STATE_HIGH || bt_rssi_state == > > > > > > > BTC_RSSI_STATE_STAY_HIGH" is true, because the 'then' and 'else' > > > > > > > branches are identical. Should one of the branches be modified, or the > > > > > > > entire 'if' statement replaced? > > > > > > > # 2945| } > > > > > > > # 2946| > > > > > > > # 2947|-> if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) || > > > > > > > # 2948| (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) > > > > > > > # 2949| btc8821a2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 26); > > > > > > > > > > > > > > Error: IDENTICAL_BRANCHES (CWE-398): [#def214] > > > > > > > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3135: > > > > > > > identical_branches: The same code is executed regardless of whether > > > > > > > "wifi_bw == BTC_WIFI_BW_LEGACY" is true, because the 'then' and 'else' > > > > > > > branches are identical. Should one of the branches be modified, or the > > > > > > > entire 'if' statement replaced? > > > > > > > # 3133| btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_BW, &wifi_bw); > > > > > > > # 3134| > > > > > > > # 3135|-> if (wifi_bw == BTC_WIFI_BW_LEGACY) { > > > > > > > # 3136| /* for HID at 11b/g mode */ > > > > > > > # 3137| btc8821a2ant_coex_table(btcoexist, NORMAL_EXEC, 0x55ff55ff, > > > > > > > > > > > > > > Error: IDENTICAL_BRANCHES (CWE-398): [#def215] > > > > > > > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3324: > > > > > > > identical_branches: The same code is executed regardless of whether > > > > > > > "bt_rssi_state == BTC_RSSI_STATE_HIGH || bt_rssi_state == > > > > > > > BTC_RSSI_STATE_STAY_HIGH" is true, because the 'then' and 'else' > > > > > > > branches are identical. Should one of the branches be modified, or the > > > > > > > entire 'if' statement replaced? > > > > > > > # 3322| } > > > > > > > # 3323| > > > > > > > # 3324|-> if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) || > > > > > > > # 3325| (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) { > > > > > > > # 3326| btc8821a2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 23); > > > > > > > > > > > > > > > > > > > > > In my opinion, they seem to be real bugs. However, it's very difficult > > > > > > > to imagine what actions must be taken on each branch of the if-else > > > > > > > because they strongly depend on magic numbers, which are different > > > > > > > configurations for the hw, I guess. > > > > > > > > > > > > > > Can the maintainers confirm if these are real bugs and see how to fix them? > > > > > > > > > > > > > > Regards > > > > > > > -- > > > > > > > Íñigo Huguet > > > > > > > > > > > > Hello, > > > > > > > > > > > > A few weeks ago I sent the message above notifying a potential bug in > > > > > > rtlwifi module. I just wanted to be sure that it has been received. > > > > > > Can the maintainers acknowledge whether they have seen it? > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > Not real bugs. The coexistence programmers preserve the same code of > > > > > branches intentionally to fine tune performance easier, because bandwidth and > > > > > RSSI strength are highly related to coexistence performance. > > > > > The basic rule of performance tuning is to assign most time slot to BT > > > > > for realtime application, and WiFi uses remaining time slot but don't lower > > > > > than low bound. > > > > > > > > > > > > > > > > Hi, > > > > > > I can add comments. Do you need any keyword within comment to avoid your > > > checking tool warns this false alarm? > > > > > I do "git grep coverity | wc -l" and there are only 8 instances. > I'm not sure if I can add comments with "coverity" marker. > > -- > Ping-Ke > -- Íñigo Huguet