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 F26A6A0524; Tue, 13 Apr 2021 01:08:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 676DC160850; Tue, 13 Apr 2021 01:08:36 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 6FBEA4067E for ; Tue, 13 Apr 2021 01:08:34 +0200 (CEST) IronPort-SDR: MtYo6yLOXlW1QRFnyUZ5C7aa45rkjDlY6j33J9/L+ZhmTMiWlAPlPDHF4SPmkt5Nk+a73lFPHP dR7/LeaWHJGw== X-IronPort-AV: E=McAfee;i="6200,9189,9952"; a="194408273" X-IronPort-AV: E=Sophos;i="5.82,216,1613462400"; d="scan'208";a="194408273" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Apr 2021 16:08:29 -0700 IronPort-SDR: uPJ38kSxLfE5TOczqFVKyVCAjY0fO9Oqu500rXrLiofaBDwsEpXdqMylsSKV3mC8ImIszDpb0v oXKqEpfzLgsw== X-IronPort-AV: E=Sophos;i="5.82,216,1613462400"; d="scan'208";a="398548456" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.218.133]) ([10.213.218.133]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Apr 2021 16:08:27 -0700 To: "Min Hu (Connor)" , dev@dpdk.org Cc: thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru References: <1618046334-39857-1-git-send-email-humin29@huawei.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <41dda92b-e8d8-8e52-24b8-5b7598c9a6ec@intel.com> Date: Tue, 13 Apr 2021 00:08:22 +0100 MIME-Version: 1.0 In-Reply-To: <1618046334-39857-1-git-send-email-humin29@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] ethdev: add sanity checks in control APIs 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/10/2021 10:18 AM, Min Hu (Connor) wrote: > This patch adds more sanity checks in control path APIs. > > Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input") > Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables") > Fixes: 0366137722a0 ("ethdev: check for invalid device name") > Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process model") > Fixes: 5b7ba31148a8 ("ethdev: add port ownership") > Fixes: f8244c6399d9 ("ethdev: increase port id range") > Cc: stable@dpdk.org > > Signed-off-by: Min Hu (Connor) Thanks Connor for the patch, please find a few minor comments below. <...> > @@ -3822,6 +3903,9 @@ rte_eth_dev_rss_reta_query(uint16_t port_id, > struct rte_eth_dev *dev; > int ret; > > + if (reta_conf == NULL) > + return -EINVAL; > + The 'eth_check_reta_mask()' & 'eth_check_reta_entry()' static fucntions are also checking 'reta_conf' against NULL, they may be removed after these checks. <...> > @@ -4447,6 +4552,9 @@ rte_eth_dev_callback_process(struct rte_eth_dev *dev, > struct rte_eth_dev_callback dev_cb; > int rc = 0; > > + if (dev == NULL) > + return -EINVAL; > + This is internal API, not sure if needed to verify 'dev' here, it won't hurt of course, but just to prevent unnecessary checks. No strong option. Same comment for all internal functions that changed. <...> > @@ -5370,6 +5502,9 @@ rte_eth_dev_get_dcb_info(uint16_t port_id, > { > struct rte_eth_dev *dev; > > + if (dcb_info == NULL) > + return -EINVAL; > + Function documentation in the header file, doesn't list '-EINVAL' as a return value, whenever a new return value is returned, it needs to be documented.