[dpdk-dev] [PATCH 2/2] ethdev: use strlcpy instead of snprintf on initialization

[Ferruh Yigit]

On 3/4/2019 7:12 PM, Stephen Hemminger wrote:
> On Mon, 4 Mar 2019 12:11:20 +0300
> Andrew Rybchenko <arybchenko at solarflare.com> wrote:
> 
>> On 3/1/19 9:42 PM, Stephen Hemminger wrote:
>>> On Fri, 1 Mar 2019 10:48:58 +0300
>>> Andrew Rybchenko <arybchenko at solarflare.com> wrote:
>>>  
>>>> On 3/1/19 1:47 AM, Stephen Hemminger wrote:  
>>>>> Don't need to use snprintf for simple name copy.
>>>>>
>>>>> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
>>>>> ---
>>>>>    lib/librte_ethdev/rte_ethdev.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>>>> index 95889ed206db..8bd54dcf58c1 100644
>>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>>> @@ -459,7 +459,7 @@ rte_eth_dev_allocate(const char *name)
>>>>>    	}
>>>>>    
>>>>>    	eth_dev = eth_dev_get(port_id);
>>>>> -	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
>>>>> +	strlcpy(eth_dev->data->name, name, RTE_ETH_NAME_MAX_LEN);  
>>>> Why is sizeof() substituted with RTE_ETH_NAME_MAX_LEN?  
>>> Same thing, I just wanted to make the length obvious to the reader.
>>>  
>>>> I thought that sizeof() is the first choice in such cases since it is a
>>>> bit more
>>>> safer vs possible changes in the code.
>>>>
>>>> BTW, wouldn't it be more friendly to check name length on entry and
>>>> reject if it is too long? (and same for rte_eth_dev_create())  
>>> It is impossible for name to long since since both structures are the same.  
>>
>> Which structures? name is an input parameter of the function.
>>
> 
> Sorry my confusion, that was in patch 1.

Only concern is keep using "sizeof(eth_dev->data->name)" or
RTE_ETH_NAME_MAX_LEN, both are correct but I agree with Andrew about using sizeof.


Stephen,

Would you be OK if I update it to sizeof() while merging, so I can merge them?

Thanks,
ferruh