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 47AB7A0C40; Wed, 7 Apr 2021 18:10:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F0801140F37; Wed, 7 Apr 2021 18:10:08 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id A8E88140F23 for ; Wed, 7 Apr 2021 18:10:07 +0200 (CEST) IronPort-SDR: jA38o4Z/TTuRZ7ngweehf/9muH0BbeINRnspZcJRqFebZqaBL6O7EkOMazX9B0vwconjxn7TIo xTPHw65usVxA== X-IronPort-AV: E=McAfee;i="6000,8403,9947"; a="254681152" X-IronPort-AV: E=Sophos;i="5.82,203,1613462400"; d="scan'208";a="254681152" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Apr 2021 09:10:06 -0700 IronPort-SDR: 9gHRKTLfs4bRP5ULQlXBy/fuQh0apMJxngoFCBev8FxRcyIBL3eX3CYqNqJSVAx3iPwui1oPHd /Qe4X+bKc2tw== X-IronPort-AV: E=Sophos;i="5.82,203,1613462400"; d="scan'208";a="379877785" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.202.196]) ([10.213.202.196]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Apr 2021 09:10:03 -0700 To: 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> From: Ferruh Yigit X-User: ferruhy Message-ID: <68bb19fb-2d1a-677d-05f2-e2029d5095a5@intel.com> Date: Wed, 7 Apr 2021 17:10:00 +0100 MIME-Version: 1.0 In-Reply-To: <39bb5d09-9e95-db2d-929f-b0b3e922d921@oss.nxp.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 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?