From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <ferruh.yigit@intel.com>, hemant.agrawal@nxp.com,
Ajit Khaparde <ajit.khaparde@broadcom.com>,
Jerin Jacob <jerinjacobk@gmail.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <Andrew.Rybchenko@oktetlabs.ru>,
"Min Hu (Connor)" <humin29@huawei.com>, "dev@dpdk.org" <dev@dpdk.org>,
"olivier.matz@6wind.com" <olivier.matz@6wind.com>,
"david.marchand@redhat.com" <david.marchand@redhat.com>,
"jerinj@marvell.com" <jerinj@marvell.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>
References: <6114bde2-423a-da82-ac4d-608141235e39@huawei.com>
<1672555.D3d3fyF7jD@thomas>
<DM6PR11MB44917D38667EB1A6B2E5F0BA9A759@DM6PR11MB4491.namprd11.prod.outlook.com>
<CALBAE1MY4ijoJqzCeaS+Ud-X0LQ1BzX8ypbVW043GJgcyUZEiQ@mail.gmail.com>
<CACZ4nhuqA3g5UCKS=sDT34rK-r6yP4fOwtPC7NdFKwpUn8+FuQ@mail.gmail.com>
<39bb5d09-9e95-db2d-929f-b0b3e922d921@oss.nxp.com>
<68bb19fb-2d1a-677d-05f2-e2029d5095a5@intel.com>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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 <jerinjacobk@gmail.com>
>>> wrote:
>>>> On Wed, Apr 7, 2021 at 5:23 PM Ananyev, Konstantin
>>>> <konstantin.ananyev@intel.com> 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