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 DAFADA0C4B; Fri, 19 Nov 2021 10:56:41 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 67CC340143; Fri, 19 Nov 2021 10:56:41 +0100 (CET) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by mails.dpdk.org (Postfix) with ESMTP id 0899A40140; Fri, 19 Nov 2021 10:56:40 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 88D7A5C0152; Fri, 19 Nov 2021 04:56:39 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 19 Nov 2021 04:56:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= vTm/mFZQ3eor9yeF2W8/l8gqP0smSGXlOpMbfifwHp8=; b=TeV7L03WHb7Vwxra AWNQqhS/oXSeNxRBNcRxkpOGNxar8+0SjxSQGwL3Mk66XAZ3F7TIZ3YzRXh0Y/ng BIBaMZqoKyd9rPsjgIbOtlowQHzzPWPD+Bn+loTo+mBzkybEpg1PHUcO3QIEt1aB fX475AiwZEKYtg4k/nSmarjphD7aN+qOPnJn4Tk9vhE9axdX0fgoYMK9xscbDSuS wkVjCbOIYkNpQ2rS0hJMW9pKI5cl/bcs+ECTob7gSl6XrvH7VW809ymFx+veW76e iX8tSrpVALWp5BghbRno2ZH8Y7MpHJJsbMjyEskOISx5kLZ8/m15n5XQE/IV0iWk u263+w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=vTm/mFZQ3eor9yeF2W8/l8gqP0smSGXlOpMbfifwH p8=; b=GYzdBarvNSFCnwjt/izB9gogk5x99pY0zo5RmfdJIgWt8j5z53N+2Zm8l AX50dK5VLReW8KJXNfiXfPasFS93TmxdrlvAa+4sL9CJ8Jpa3qBS7AHgTDZA8JPt idjjsOAiBOAewH5/OAho8CqSpLJmD+nbOxMsBLQYput5ZGRNPUehJ3hFuQqzrMKk eG5bjMszmu3OjlWrI2L/9Uq1lJ9jJRiicxSB/q/kWZW4vLZOgQ94tcmss/hprY4d uwzl7Ggclgbu/LvaR3Y4NxCNcAepBrdSl1xt60qMvCblz4nG01xVBAUXCHFNJzMW Ow3RaSZ6X8LZnYMlpMZWl2oO2PCLg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrfeekgddtlecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 19 Nov 2021 04:56:38 -0500 (EST) From: Thomas Monjalon To: Tyler Retzlaff Cc: eagostini@nvidia.com, techboard@dpdk.org, dev@dpdk.org, Andrew Rybchenko , David Marchand , Ferruh Yigit Subject: Re: [PATCH v1] gpudev: return EINVAL if invalid input pointer for free and unregister Date: Fri, 19 Nov 2021 10:56:36 +0100 Message-ID: <4933677.M48bl325bv@thomas> In-Reply-To: References: <20211118192802.23955-1-eagostini@nvidia.com> <20211118201931.GA6492@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 19/11/2021 10:34, Ferruh Yigit: > On 11/18/2021 8:19 PM, Tyler Retzlaff wrote: > > On Thu, Nov 18, 2021 at 07:28:02PM +0000, eagostini@nvidia.com wrote: > >> From: Elena Agostini > >> > >> Signed-off-by: Elena Agostini > >> --- > >> lib/gpudev/gpudev.c | 10 ++++++++++ > >> lib/gpudev/rte_gpudev.h | 2 ++ > >> 2 files changed, 12 insertions(+) > >> > >> diff --git a/lib/gpudev/gpudev.c b/lib/gpudev/gpudev.c > >> index 2b174d8bd5..97575ed979 100644 > >> --- a/lib/gpudev/gpudev.c > >> +++ b/lib/gpudev/gpudev.c > >> @@ -576,6 +576,11 @@ rte_gpu_mem_free(int16_t dev_id, void *ptr) > >> return -rte_errno; > >> } > >> > >> + if (ptr == NULL) { > >> + rte_errno = EINVAL; > >> + return -rte_errno; > >> + } > > > > in general dpdk has real problems with how it indicates that an error > > occurred and what error occurred consistently. > > > > some api's return 0 on success > > and maybe return -errno if ! 0 > > and maybe return errno if ! 0 Which function returns a positive errno? > > and maybe set rte_errno if ! 0 > > > > some api's return -1 on failure > > and set rte_errno if -1 > > > > some api's return < 0 on failure > > and maybe set rte_errno > > and maybe return -errno > > and maybe set rte_errno and return -rte_errno > > This is a generic comment, cc'ed a few more folks to make the comment more > visible. > > > this isn't isiolated to only this change but since additions and context > > in this patch highlight it maybe it's a good time to bring it up. > > > > it's frustrating to have to carefully read the implementation every time > > you want to make a function call to make sure you're handling the flavor > > of error reporting for a particular function. > > > > if this is new code could we please clearly identify the current best > > practice and follow it as a standard going forward for all new public > > apis. I think this patch is following the best practice. 1/ Return negative value in case of error 2/ Set rte_errno 3/ Set same absolute value in rte_errno and return code