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 0BFFFA0579; Thu, 8 Apr 2021 03:06:43 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 85A16140F24; Thu, 8 Apr 2021 03:06:42 +0200 (CEST) Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) by mails.dpdk.org (Postfix) with ESMTP id 7853940698 for ; Thu, 8 Apr 2021 03:06:40 +0200 (CEST) Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4FG32V5v9tzkjD0; Thu, 8 Apr 2021 09:04:50 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS405-HUB.china.huawei.com (10.3.19.205) with Microsoft SMTP Server id 14.3.498.0; Thu, 8 Apr 2021 09:06:33 +0800 To: "Burakov, Anatoly" , Ferruh Yigit , , Ajit Khaparde , Jerin Jacob CC: "Ananyev, Konstantin" , Thomas Monjalon , 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> <1672555.D3d3fyF7jD@thomas> <39bb5d09-9e95-db2d-929f-b0b3e922d921@oss.nxp.com> <68bb19fb-2d1a-677d-05f2-e2029d5095a5@intel.com> <3865dae6-1245-c2be-7b9c-3eb6e1a8c0d4@intel.com> From: "Min Hu (Connor)" Message-ID: Date: Thu, 8 Apr 2021 09:06:33 +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: <3865dae6-1245-c2be-7b9c-3eb6e1a8c0d4@intel.com> 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" 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 ? Thanks. 在 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 >