From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 1C326A0503;
Fri, 20 May 2022 11:14:48 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id BB341427ED;
Fri, 20 May 2022 11:14:47 +0200 (CEST)
Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187])
by mails.dpdk.org (Postfix) with ESMTP id 8D95B40222
for <dev@dpdk.org>; Fri, 20 May 2022 11:14:46 +0200 (CEST)
Received: from kwepemi500012.china.huawei.com (unknown [172.30.72.53])
by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4L4LcG6ncczfbNG;
Fri, 20 May 2022 17:13:18 +0800 (CST)
Received: from [10.67.103.128] (10.67.103.128) by
kwepemi500012.china.huawei.com (7.221.188.12) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.2375.24; Fri, 20 May 2022 17:14:42 +0800
Subject: Re: [PATCH 05/12] net/hns3: fix an unreasonable memset
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>, <dev@dpdk.org>
CC: <ferruh.yigit@intel.com>, <thomas@monjalon.net>
References: <20220519122917.2334-1-humin29@huawei.com>
<20220519122917.2334-6-humin29@huawei.com>
<1c148094-3550-b78e-7c23-1819582a83ff@oktetlabs.ru>
<19acdf78-af14-25fa-a52b-8b1f96d3fcb3@huawei.com>
<02c76cd2-da56-7619-d52a-fa997bc81106@oktetlabs.ru>
From: "Min Hu (Connor)" <humin29@huawei.com>
Message-ID: <3024730f-ca6a-e82e-2c7d-80c41d4fd6ad@huawei.com>
Date: Fri, 20 May 2022 17:14:42 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
Thunderbird/68.3.1
MIME-Version: 1.0
In-Reply-To: <02c76cd2-da56-7619-d52a-fa997bc81106@oktetlabs.ru>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.67.103.128]
X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To
kwepemi500012.china.huawei.com (7.221.188.12)
X-CFilter-Loop: Reflected
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Hi,Andrew,
在 2022/5/20 16:59, Andrew Rybchenko 写道:
> On 5/20/22 11:37, Min Hu (Connor) wrote:
>> Hi, Andrew ,
>>
>> 在 2022/5/20 15:58, Andrew Rybchenko 写道:
>>> On 5/19/22 15:29, Min Hu (Connor) wrote:
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> This patch fixes an unreasonable memset.
>>>>
>>>> Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> ---
>>>> drivers/net/hns3/hns3_rxtx.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/hns3/hns3_rxtx.c
>>>> b/drivers/net/hns3/hns3_rxtx.c
>>>> index 510802be05..5a2cfe5a54 100644
>>>> --- a/drivers/net/hns3/hns3_rxtx.c
>>>> +++ b/drivers/net/hns3/hns3_rxtx.c
>>>> @@ -776,7 +776,7 @@ hns3vf_reset_all_tqps(struct hns3_hw *hw)
>>>> int ret;
>>>> uint16_t i;
>>>> - memset(msg_data, 0, sizeof(uint16_t));
>>>> + memset(msg_data, 0, sizeof(msg_data));
>>>
>>> It looks a bit suspicious. May be it is better to do:
>>> memset(&msg_data, 0, sizeof(msg_data));
>> I think this is too hard to understand for &msg_data is **p.
>> maybe it confuse others when use memset to operate level-2 pointer.
>
> msg_data == &amsg_data
> since it is an array
Yes, you are right, the address of 'msg_data ' is the same
as the address of '&amsg_data'.
But I still think this usage is not common.
like the current code in DPDK:
*****
static int
app_parse_flow_conf(const char *conf_str)
{
int ret;
uint32_t vals[5];
struct flow_conf *pconf;
uint64_t mask;
memset(vals, 0, sizeof(vals));
...
*****
how about others's opinion? @Ferruh, @Thomas.
>
>>
>>
>>> It is pretty common mistake to do:
>>> memset(p, 0, sizeof(p));
>>> instead of
>>> memset(p, 0, sizeof(*p));
>>> when p is a pointer.
>>> I realize that msg_data is an array in this case, but
>>> I think it is better to avoid bad pattern in the code.
>>>
>>>> ret = hns3_send_mbx_msg(hw, HNS3_MBX_QUEUE_RESET, 0, msg_data,
>>>> sizeof(msg_data), true, &reset_status,
>>>> sizeof(reset_status));
>>>
>>> .
>
> .