RE: [PATCH v2] wifi: rtw89: 8852b: avoid reporting errors by cppcheck

[Ping-Ke Shih <pkshih@realtek.com>]

> -----Original Message-----
> From: Li Lin Mao <lilinmao@kylinos.cn>
> Sent: Wednesday, January 10, 2024 12:18 PM
> To: linux-wireless@vger.kernel.org
> Cc: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org; Li Lin Mao <lilinmao@kylinos.cn>
> Subject: [PATCH v2] wifi: rtw89: 8852b: avoid reporting errors by cppcheck

The subject should specify what you are fixing. If you send two patches to
correct two different cppcheck errors, how can we address their differences
by subject? I mean your subject is too common, like checkpatch.pl will
throw out warning if cppcheck is added:

# Check email subject for common tools that don't need to be mentioned
                if ($in_header_lines &&
                    $line =~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b[^:]/i) {
                        WARN("EMAIL_SUBJECT",
                             "A patch subject line should describe the change not the tool that found it\n" . $herecurr);
                }

> 
> Due to some reasons in cppcheck, the following issues might be reported:

I think you can mention the "reason" -- out of bounds. 

> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1414:22: error: Array
>  'iqk_info->iqk_mcc_ch[2][4]' accessed at index iqk_info->iqk_mcc_ch[2][*],
> which is out of bounds. [arrayIndexOutOfBounds]
>  iqk_info->iqk_mcc_ch[idx][path] = chan->channel;
>                      ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1393:2: note: After for
> loop, idx has value 2
>  for (idx = 0; idx < RTW89_IQK_CHS_NR; idx++) {
>  ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1414:22: note: Array index
> out of bounds
>  iqk_info->iqk_mcc_ch[idx][path] = chan->channel;
>                      ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1424:38: error: Array
> 'iqk_info->iqk_mcc_ch[2][4]' accessed at index iqk_info->iqk_mcc_ch[2][*],
> which is out of bounds. [arrayIndexOutOfBounds]
>       idx, path, iqk_info->iqk_mcc_ch[idx][path]);
>                                      ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1393:2: note: After for
> loop, idx has value 2
>  for (idx = 0; idx < RTW89_IQK_CHS_NR; idx++) {
>  ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1424:38: note: Array index
> out of bounds
>       idx, path, iqk_info->iqk_mcc_ch[idx][path]);
>                                      ^
> But actually this might be a false alarm. We avoided it in some way.
> 
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

Sorry. I have not signed this patch. Please remove my s-o-b.

> Signed-off-by: Li Lin Mao <lilinmao@kylinos.cn>
> ---
>  drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c
> b/drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c
> index 259df67836a0..03169287667c 100644
> --- a/drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c
> +++ b/drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c
> @@ -1388,17 +1388,15 @@ static void _iqk_get_ch_info(struct rtw89_dev *rtwdev, enum rtw89_phy_idx phy, u
>         u32 reg_rf18;
>         u32 reg_35c;
>         u8 idx;
> -       u8 get_empty_table = false;
> 
>         for (idx = 0; idx < RTW89_IQK_CHS_NR; idx++) {
>                 if (iqk_info->iqk_mcc_ch[idx][path] == 0) {
> -                       get_empty_table = true;
>                         break;
>                 }
>         }
>         rtw89_debug(rtwdev, RTW89_DBG_RFK, "[IQK] (1)idx = %x\n", idx);
> 
> -       if (!get_empty_table) {
> +       if (idx >= RTW89_IQK_CHS_NR) {
>                 idx = iqk_info->iqk_table_idx[path] + 1;
>                 if (idx > 1)
>                         idx = 0;

rtw8852a has similar problem. Doesn't cppcheck report the same warning?