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 30AD4A0C43; Wed, 7 Apr 2021 18:27:04 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C1006140ECE; Wed, 7 Apr 2021 18:27:03 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id 0A403140EBA for ; Wed, 7 Apr 2021 18:27:01 +0200 (CEST) IronPort-SDR: 4rW8G9jqo5EzKbA6pyuHta7h1kgYHYeGXrsOzW/gHFlkoIKrIdlK78Cnl9y3qz7C/pACUZ0SXl aaL9G2k3vLxA== X-IronPort-AV: E=McAfee;i="6000,8403,9947"; a="213739794" X-IronPort-AV: E=Sophos;i="5.82,203,1613462400"; d="scan'208";a="213739794" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Apr 2021 09:27:00 -0700 IronPort-SDR: 5MPgV++n1NLuCsU8IcUpgcSxSoGegeHDiFas6ao4oRkdIFPcwUnGwTDyNvLRSA4QxsBvVvS9tA jTAU/dp9dIsA== X-IronPort-AV: E=Sophos;i="5.82,203,1613462400"; d="scan'208";a="530264994" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.215.35]) ([10.213.215.35]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Apr 2021 09:26:51 -0700 To: Ferruh Yigit , hemant.agrawal@nxp.com, Ajit Khaparde , Jerin Jacob Cc: "Ananyev, Konstantin" , Thomas Monjalon , Andrew Rybchenko , "Min Hu (Connor)" , "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> From: "Burakov, Anatoly" Message-ID: <3865dae6-1245-c2be-7b9c-3eb6e1a8c0d4@intel.com> Date: Wed, 7 Apr 2021 17:26:42 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <68bb19fb-2d1a-677d-05f2-e2029d5095a5@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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 -- Thanks, Anatoly