Received-SPF: pass (google.com: domain of linux-wireless+bounces-1649-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Ping-Ke Shih <pkshih@realtek.com>
To: Li Lin Mao <lilinmao@kylinos.cn>,
        "linux-wireless@vger.kernel.org"
	<linux-wireless@vger.kernel.org>
CC: "kvalo@kernel.org" <kvalo@kernel.org>
Subject: RE: [PATCH v2] wifi: rtw89: 8852b: avoid reporting errors by cppcheck
Thread-Topic: [PATCH v2] wifi: rtw89: 8852b: avoid reporting errors by
 cppcheck
Thread-Index: AQHaQ3v5NQKCFMN/G0mgePLSUJwh3rDSh6og
Date: Wed, 10 Jan 2024 05:48:28 +0000
Message-ID: <92f7e7f317bf4e6bb68ce385b0c1e119@realtek.com>
References: <20240110041736.2809393-1-lilinmao@kylinos.cn>
In-Reply-To: <20240110041736.2809393-1-lilinmao@kylinos.cn>
Accept-Language: en-US, zh-TW
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0


> -----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 <lili=
nmao@kylinos.cn>
> Subject: [PATCH v2] wifi: rtw89: 8852b: avoid reporting errors by cppchec=
k

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 =3D~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b=
[^:]/i) {
                        WARN("EMAIL_SUBJECT",
                             "A patch subject line should describe the chan=
ge not the tool that found it\n" . $herecurr);
                }

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

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

> 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] =3D chan->channel;
>                      ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1393:2: note: After for
> loop, idx has value 2
>  for (idx =3D 0; idx < RTW89_IQK_CHS_NR; idx++) {
>  ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1414:22: note: Array in=
dex
> out of bounds
>  iqk_info->iqk_mcc_ch[idx][path] =3D 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 =3D 0; idx < RTW89_IQK_CHS_NR; idx++) {
>  ^
> drivers/net/wireless/realtek/rtw89/rtw8852b_rfk.c:1424:38: note: Array in=
dex
> 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.
>=20
> 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(-)
>=20
> 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 *rt=
wdev, enum rtw89_phy_idx phy, u
>         u32 reg_rf18;
>         u32 reg_35c;
>         u8 idx;
> -       u8 get_empty_table =3D false;
>=20
>         for (idx =3D 0; idx < RTW89_IQK_CHS_NR; idx++) {
>                 if (iqk_info->iqk_mcc_ch[idx][path] =3D=3D 0) {
> -                       get_empty_table =3D true;
>                         break;
>                 }
>         }
>         rtw89_debug(rtwdev, RTW89_DBG_RFK, "[IQK] (1)idx =3D %x\n", idx);
>=20
> -       if (!get_empty_table) {
> +       if (idx >=3D RTW89_IQK_CHS_NR) {
>                 idx =3D iqk_info->iqk_table_idx[path] + 1;
>                 if (idx > 1)
>                         idx =3D 0;

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