Re: [PATCH 05/12] net/hns3: fix an unreasonable memset

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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

> 
> 
>> 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));
>>
>> .