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 5EB67A0579; Thu, 8 Apr 2021 10:23:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4BE56141106; Thu, 8 Apr 2021 10:23:01 +0200 (CEST) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by mails.dpdk.org (Postfix) with ESMTP id 38E80141105 for ; Thu, 8 Apr 2021 10:23:00 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id A9C5C5803E5; Thu, 8 Apr 2021 04:22:59 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Thu, 08 Apr 2021 04:22:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm3; bh= IQg4eOKKCCXF6yiLQAtcewLumKzZMq1J52QTWeLqDWE=; b=NOTbdjzut7ZIRKYR IOWH2M6Y34Z9ZG/708ToTVbIgZu1KXyZxHV3sBpTzk7vleQmh3oxNvW34l2pNDYv ePA7piYAB1hjxtKEUL6IGlePzvSb52woQxfQVFKvtwriJB619M4uXT91NfG1tem9 TRsx3Nu6mKcVBhlCkPpEogbrFXrHuTFEej6VXTJhviT6bdqaTd3LrOWoEFKRzuAK SnMvgZWD2ABCqqPyNfW5o9bSmutDPFlZu+YltNG9troJ/wnNcXZxuFVD9ekmFnHZ LjZ2eumVexRW1B78m3LLnGN8B1FcY+gltD4nHo1hJkAvjso8fs/1Q5PpDy/jckFy zpohWQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=IQg4eOKKCCXF6yiLQAtcewLumKzZMq1J52QTWeLqD WE=; b=kYQ50kIroSV7/wEuxdel7/J0ai/heVWY0n7gnL+lZGO+fKAaEnwheheAH xRjQ6y3Yeas5BIFV9+mroIqnPQkxWyfa2/AE2HedjxMXtC+cPanBECo0hSHU670N vqtnWIsAR8VKcY0j3V/R5wrN3nAh+Prrh+FpXBDQ++l2Z1yu2/p1n6Fr7nVJOS1M PIlwagiY5FJ/u/s4v8vQCmglRDOpxbnx89Epi8Cg3HoKlOO2uyiAE+BJdKJq1KAd 4RM6gh8VoJ2HxwWRbKRCHNwU5eHH5tt0OxTQq4adTklMunF+sQZJtEtBkKUt0BvG rpNOUERb5n1i96X3GEqSnw8TruSvw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrudejledgtdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepkeethedtieevhfeigeejleegudefjeehkeekteeuveeiuedvveeu tdejveehveetnecukfhppeejjedrudefgedrvddtfedrudekgeenucevlhhushhtvghruf hiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghl ohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 2C79F24005A; Thu, 8 Apr 2021 04:22:57 -0400 (EDT) From: Thomas Monjalon To: "Burakov, Anatoly" , Ferruh Yigit , hemant.agrawal@nxp.com, Ajit Khaparde , Jerin Jacob , "Min Hu (Connor)" Cc: "Ananyev, Konstantin" , Andrew Rybchenko , "dev@dpdk.org" , "olivier.matz@6wind.com" , "david.marchand@redhat.com" , "jerinj@marvell.com" , "Richardson, Bruce" Date: Thu, 08 Apr 2021 10:22:55 +0200 Message-ID: <4281358.EOTebcGJYR@thomas> In-Reply-To: References: <6114bde2-423a-da82-ac4d-608141235e39@huawei.com> <3865dae6-1245-c2be-7b9c-3eb6e1a8c0d4@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" 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" 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. >=20 > 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. >=20 > 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 > =E5=9C=A8 2021/4/8 0:26, Burakov, Anatoly =E5=86=99=E9=81=93: > > 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 =20 > >>>> 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=20 > >>>>>>>> *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=20 > >>>>>>>> *stats) > >>>>>>>> { > >>>>>>>> if (stats =3D=3D NULL) > >>>>>>>> return -EINVAL; > >>>>>>>> ... > >>>>>>>> } > >>>>>>>> > >>>>>>>> Or, that is redundant, the parameter correctness should be=20 > >>>>>>>> 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= =20 > >> path APIs? > >> > >=20 > > +1 > >=20 > > Don't have anything useful to add that hasn't already been said, but=20 > > seems like a nice +1-train going on here, so i thought i'd hop on board= :D > >=20 >=20