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 3181941BB2; Thu, 2 Feb 2023 21:26:07 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B644440EDC; Thu, 2 Feb 2023 21:26:06 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 3C12E40693 for ; Thu, 2 Feb 2023 21:26:05 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 7054F20B7102; Thu, 2 Feb 2023 12:26:04 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 7054F20B7102 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1675369564; bh=p/F5MXsAF7BsnUgXKYNPQ41bZRqtsLb5Ilpaenu8il4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Cq4YjOJ3AogpRD8RFgd9oexmhM75shD03h3lLTT3GYToGqGkivj1DGlUnh1S7Oh1M qWFXQa0FFXwadcqKtONLdWi6CBpwkfvXUgXn7Kl/0HHRRCq196eHT4N6L03I6BirHM glIPnfWbkB3P5rLTiEpjZbq/JCJAmyeLp+Y5+gXU= Date: Thu, 2 Feb 2023 12:26:04 -0800 From: Tyler Retzlaff To: Ben Magistro Cc: Olivier Matz , Thomas Monjalon , ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, ben.magistro@trinitycyber.com, dev@dpdk.org, Stefan Baranoff Subject: Re: Sign changes through function signatures Message-ID: <20230202202604.GA23284@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Feb 02, 2023 at 02:23:39PM -0500, Ben Magistro wrote: > Hello, > > While making some updates to our code base for 22.11.1 that were missed in > our first pass through, we hit the numa node change[1]. In the process of > updating our code, we noticed that a couple functions (rx/tx_queue_setup, > maybe more that we aren't using) state they accept `SOCKET_ID_ANY` but the > function signature then asks for an unsigned integer while `SOCKET_ID_ANY` > is `-1`. Following it through the redirect to the "real" function it also > asks for an unsigned integer which is then passed on to one or more > functions asking for an integer. As an example using the the i40e driver > -- we would call `rte_eth_tx_queue_setup` [2] which ultimately calls > `i40e_dev_tx_queue_setup`[3] which finally calls `rte_zmalloc_socket`[4] > and `rte_eth_dma_zone_reserve`[5]. > > I guess what I am looking for is clarification on if this is intentional or > if this is additional cleanup that may need to be completed/be desirable so > that signs are maintained through the call paths and avoid potentially > producing sign-conversion warnings. From the very quick glance I took at > the i40e driver, it seems these are just passed through to other functions > and no direct use/manipulation occurs (at least in the mentioned functions). i believe this is just sloppyness with sign in our api surface. i too find it frustrating that use of these api force either explicit casts or suffer having to suppress warnings. in the past examples of this have been cleaned up without full deprecation notices but there are a lot of instances. i also feel (unpopular opinion) that for some integer types like this that have constrained range / number spaces it would be of value to introduce a typedef that can be used consistently. for now you'll just have to add the casts and hopefully in the future we will fix the api making them unnecessary. of course feel free to submit patches too, it would be great to have these cleaned up. > > 1) > https://patches.dpdk.org/project/dpdk/patch/20221004145850.32331-1-olivier.matz@6wind.com/ > 2) > https://doc.dpdk.org/api/rte__ethdev_8h.html#a796c2f20778984c6f41b271e36bae50e > 3) https://github.com/DPDK/dpdk/blob/main/drivers/net/i40e/i40e_rxtx.c#L1949 > 4) > https://doc.dpdk.org/api/rte__malloc_8h.html#a7e9f76b7e0b0921a617c6ab8b28f53b3 > 5) > https://github.com/DPDK/dpdk/blob/1094dd940ec0cc4e3ce2c5cd94807350855a17f9/lib/ethdev/ethdev_driver.h#L1566