Re: [dpdk-dev] Questions about API with no parameter check

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

在 2021/4/8 16:22, Thomas Monjalon 写道:
> 08/04/2021 03:06, Min Hu (Connor):
>> Thanks all,
>> Well, Most people who replied support input verification for APIs.
>> As the APIs are in control path APIs, so checking all input is OK.
>>
>> This is a large project because there are so many APIs and libs in DPDK.
>> I will send a set of patches to fix that.
>>
>> Thomas, Ferruh, and others, any opinions ?
> 
> Let's start with ethdev and we'll see if it looks a good addition,
> and if it is well accepted in the community.
> Thanks
> 
Got it, thanks Thomas. I will send one patch to fix ethdev.
> 
>> 在 2021/4/8 0:26, Burakov, Anatoly 写道:
>>> On 07-Apr-21 5:10 PM, Ferruh Yigit wrote:
>>>> On 4/7/2021 4:25 PM, Hemant Agrawal wrote:
>>>>>
>>>>> On 4/7/2021 8:10 PM, Ajit Khaparde wrote:
>>>>>> On Wed, Apr 7, 2021 at 6:20 AM Jerin Jacob <jerinjacobk@gmail.com>
>>>>>> wrote:
>>>>>>> On Wed, Apr 7, 2021 at 5:23 PM Ananyev, Konstantin
>>>>>>> <konstantin.ananyev@intel.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> 07/04/2021 13:28, Min Hu (Connor):
>>>>>>>>>> Hi, all,
>>>>>>>>>>       Many APIs in DPDK does not check if the pointer parameter is
>>>>>>>>>> NULL or not. For example, in 'rte_ethdev.c':
>>>>>>>>>> int
>>>>>>>>>> rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>>>>>>>>>>                      uint16_t nb_rx_desc, unsigned int socket_id,
>>>>>>>>>>                      const struct rte_eth_rxconf *rx_conf,
>>>>>>>>>>                      struct rte_mempool *mp)
>>>>>>>>>>
>>>>>>>>>> int
>>>>>>>>>> rte_eth_link_get(uint16_t port_id, struct rte_eth_link *eth_link)
>>>>>>>>>>
>>>>>>>>>> int
>>>>>>>>>> rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
>>>>>>>>>>
>>>>>>>>>> int
>>>>>>>>>> rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info
>>>>>>>>>> *dev_info)
>>>>>>>>>>
>>>>>>>>>> As these APIs could be used by any APPs, if the APP give NULL as
>>>>>>>>>> the pointer parameter, segmetation default will occur.
>>>>>>>>>>
>>>>>>>>>> So, my question is, should we add check in the API? like that,
>>>>>>>>>> int rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats
>>>>>>>>>> *stats)
>>>>>>>>>> {
>>>>>>>>>>       if (stats == NULL)
>>>>>>>>>>               return -EINVAL;
>>>>>>>>>>       ...
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> Or, that is redundant, the parameter correctness should be
>>>>>>>>>> guaranteed by
>>>>>>>>>> the APP?
>>>>>>>>>>
>>>>>>>>>> What's your opinion? Hope for your reply.
>>>>>>>>> I remember it has been discussed in the past (many years ago),
>>>>>>>>> and the opinion was to not clutter the code for something that
>>>>>>>>> is a basic fault from the app.
>>>>>>>>>
>>>>>>>>> I don't have a strong opinion.
>>>>>>>>> What is your opinion? Others?
>>>>>>>> As I can see these are control path functions.
>>>>>>>> So some extra formal parameters check wouldn't hurt.
>>>>>>>> +1 from me to add them.
>>>>>>> +1 to add more sanity checks in control path APIs
>>>>>> +1
>>>>>> But are we going to check all parameters?
>>>>>
>>>>> +1
>>>>>
>>>>> It may be better to limit the number of checks.
>>>>>
>>>>
>>>> +1 to verify input for APIs.
>>>>
>>>> Why not do all, what is the downside of checking all input for control
>>>> path APIs?
>>>>
>>>
>>> +1
>>>
>>> Don't have anything useful to add that hasn't already been said, but
>>> seems like a nice +1-train going on here, so i thought i'd hop on board :D
>>>
>>
> 
> 
> 
> 
> 
> .
>