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

[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 at huawei.com>
>>>>>
>>>>> This patch fixes an unreasonable memset.
>>>>>
>>>>> Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
>>>>> Cc: stable at dpdk.org
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong at huawei.com>
>>>>> Signed-off-by: Min Hu (Connor) <humin29 at 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.

+1 to main purpose of the patch (to replace 'sizeof(uint16_t)').