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 0BF30A0524; Tue, 13 Apr 2021 05:23:37 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1FFC31609F2; Tue, 13 Apr 2021 05:23:36 +0200 (CEST) Received: from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190]) by mails.dpdk.org (Postfix) with ESMTP id 4435E1609EC for ; Tue, 13 Apr 2021 05:23:34 +0200 (CEST) Received: from DGGEMS409-HUB.china.huawei.com (unknown [172.30.72.58]) by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4FK9px3FBszpWCj; Tue, 13 Apr 2021 11:20:41 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS409-HUB.china.huawei.com (10.3.19.209) with Microsoft SMTP Server id 14.3.498.0; Tue, 13 Apr 2021 11:23:31 +0800 To: Ferruh Yigit , CC: , References: <1618046334-39857-1-git-send-email-humin29@huawei.com> <41dda92b-e8d8-8e52-24b8-5b7598c9a6ec@intel.com> From: "Min Hu (Connor)" Message-ID: Date: Tue, 13 Apr 2021 11:23:31 +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: <41dda92b-e8d8-8e52-24b8-5b7598c9a6ec@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] [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" Hi, Ferruh, Thanks for your comment. All has been fixed in v2, please check it out. Thanks. 在 2021/4/13 7:08, Ferruh Yigit 写道: > 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. > .