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 E398EA00C2;
	Wed,  8 Dec 2021 18:35:00 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 694BC4111B;
	Wed,  8 Dec 2021 18:35:00 +0100 (CET)
Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182])
 by mails.dpdk.org (Postfix) with ESMTP id E9CC1410F3;
 Wed,  8 Dec 2021 18:34:58 +0100 (CET)
Received: by linux.microsoft.com (Postfix, from userid 1086)
 id 4D87020B7179; Wed,  8 Dec 2021 09:34:56 -0800 (PST)
DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4D87020B7179
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com;
 s=default; t=1638984896;
 bh=Du2aFXs1MnmZF/9HVpJqE8NXT8usd7OjPEI6THPLAUU=;
 h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
 b=Z817xQNeLE7ZR/As/6GNi/eYiJ5Cg7+Xe6rxbfOfvL3SHauj7VJBNku5WefTkBOdm
 dMI9NHSD03hMrxghgYalsDfdDPD5NTL/EIIIb5+isswBBdkFz/ITVBhwgvRiJLfbPw
 BTadvR61lXd8gnJySPWjYN24KG4I0ygkkPnWPiN4=
Date: Wed, 8 Dec 2021 09:34:56 -0800
From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: Morten =?iso-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
 Thomas Monjalon <thomas@monjalon.net>,
 "Richardson, Bruce" <bruce.richardson@intel.com>,
 techboard@dpdk.org, dev@dpdk.org,
 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
 David Marchand <david.marchand@redhat.com>,
 "Yigit, Ferruh" <ferruh.yigit@intel.com>
Subject: Re: [PATCH v1] gpudev: return EINVAL if invalid input pointer for
 free and unregister
Message-ID: <20211208173456.GB17852@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <20211118192802.23955-1-eagostini@nvidia.com>
 <YZ5+yAwjgf6B57Np@bricha3-MOBL.ger.corp.intel.com>
 <20211201213749.GA5097@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
 <3625450.hdfAi7Kttb@thomas>
 <98CBD80474FA8B44BF855DF32C47DC35D86D33@smartserver.smartshare.dk>
 <DM6PR11MB44918CE53AAA23985A4191929A699@DM6PR11MB4491.namprd11.prod.outlook.com>
 <98CBD80474FA8B44BF855DF32C47DC35D86D34@smartserver.smartshare.dk>
 <98CBD80474FA8B44BF855DF32C47DC35D86D35@smartserver.smartshare.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D86D35@smartserver.smartshare.dk>
User-Agent: Mutt/1.5.21 (2010-09-15)
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

On Fri, Dec 03, 2021 at 11:37:10AM +0100, Morten Brørup wrote:
> > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > Sent: Thursday, 2 December 2021 14.56
> > 
> > 
> > I disagree: Negative value does not mean failure. Only -1 means
> > failure.
> > 
> > There is no -2 return value. There is no -EINVAL return value.
> > 
> > Testing for (ret < 0) might confuse someone to think that other values
> > than -1 could be returned as indication of failure, which is not the
> > case when following the convention where the functions set errno and
> > return -1 in case of failure.
> > 
> > It would be different if following a convention where the functions
> > return -errno in case of failure. In this case, testing (ret < 0) would
> > be appropriate.
> > 
> > So explicitly testing (ret == -1) clarifies which of the two
> > conventions are relevant.
> > 
> 
> I tested it on Godbolt, and (ret < 0) produces slightly smaller code than (ret == -1) on x86-64:
> 
> https://godbolt.org/z/3xME3jxq8
> 
> A binary test (Error or Data) uses 1 byte less, and a tristate test (Error, Zero or Data) uses 3 byte less.
> 
> Although there is no measurable performance difference for a single instance of this kind of test, we should consider that this kind of test appears many times in the code, so the saved bytes might add up to something slightly significant in the instruction cache.
> 
> My opinion is not so strong anymore... perhaps we should prefer performance over code readability, also in this case?
> 

i would not expect many calls that return rte_errno to be made on the
hot path. most of the use of errno / rte_errno is control but it's good
to have considered it. if i start seeing a lot of error handling in hot
paths i ordinarily find a way to get rid of it through various
techniques.