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
On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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?
Thanks!
--
Íñigo Huguet
On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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.
--
Ping-Ke
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 <[email protected]> wrote:
>
> On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> > On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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.
>
> --
> Ping-Ke
--
Íñigo Huguet
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 <[email protected]> wrote:
> >
> > On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> > > On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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?
--
Ping-Ke
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 <[email protected]> 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 <[email protected]> wrote:
> > >
> > > On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> > > > On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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?
>
> --
> Ping-Ke
>
--
Íñigo Huguet
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 <[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> > > > > On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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
Don't worry then, the explanation for humans is more than enough.
On Wed, May 5, 2021 at 4:33 PM Pkshih <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> > > > >
> > > > > On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> > > > > > On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <[email protected]> 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