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

["Min Hu (Connor)" <humin29@huawei.com>]

Hi,

在 2022/5/20 18:43, Ferruh Yigit 写道:
> On 5/20/2022 10:14 AM, Min Hu (Connor) wrote:
>> [CAUTION: External Email]
>>
>> 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.
>>
> 
> I would use array without address operator, so original patch lgtm.
thanks Ferruh.
> 
> +1 to main purpose of the patch (to replace 'sizeof(uint16_t)').
> .