From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AB5CEA0579; Thu, 8 Apr 2021 11:00:57 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 90FEC141137; Thu, 8 Apr 2021 11:00:57 +0200 (CEST) Received: from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190]) by mails.dpdk.org (Postfix) with ESMTP id 467C9141136 for ; Thu, 8 Apr 2021 11:00:55 +0200 (CEST) Received: from DGGEMS412-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4FGFXb0CcyzpWT8; Thu, 8 Apr 2021 16:58:07 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS412-HUB.china.huawei.com (10.3.19.212) with Microsoft SMTP Server id 14.3.498.0; Thu, 8 Apr 2021 17:00:50 +0800 To: Thomas Monjalon , "Burakov, Anatoly" , Ferruh Yigit , , Ajit Khaparde , "Jerin Jacob" CC: "Ananyev, Konstantin" , Andrew Rybchenko , "dev@dpdk.org" , "olivier.matz@6wind.com" , "david.marchand@redhat.com" , "jerinj@marvell.com" , "Richardson, Bruce" References: <6114bde2-423a-da82-ac4d-608141235e39@huawei.com> <3865dae6-1245-c2be-7b9c-3eb6e1a8c0d4@intel.com> <4281358.EOTebcGJYR@thomas> From: "Min Hu (Connor)" Message-ID: <5c9cf548-e824-d5b2-9b6c-a50b8c50fc7b@huawei.com> Date: Thu, 8 Apr 2021 17:00:50 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <4281358.EOTebcGJYR@thomas> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] Questions about API with no parameter check X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 在 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 >>>>>> wrote: >>>>>>> On Wed, Apr 7, 2021 at 5:23 PM Ananyev, Konstantin >>>>>>> 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 >>> >> > > > > > > . >