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 5324AA06C8; Tue, 28 Jun 2022 20:23:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E590D400D7; Tue, 28 Jun 2022 20:23:37 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 97BE640042 for ; Tue, 28 Jun 2022 20:23:36 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id AD2C020C6379; Tue, 28 Jun 2022 11:23:35 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AD2C020C6379 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1656440615; bh=EE+tGoOidXAafBuT66+u318lxqUQ/iJTgmGxlH3Ty3U=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nOG37CrqnfT33jtbfwjTZUv56jPmRYEF4R9tPJWQ7dnnHo15Q72RX/C/sZEkRNVuZ vI999szSCrGeKoyZTutn/WQHMC6b2P2NPK/Afc6QfUfcyapWP0CsnSzVedL3t7nlk6 YnqbB5L7LzviH+1pAOn5ilaql4beQifMQKdw/L0M= Date: Tue, 28 Jun 2022 11:23:35 -0700 From: Tyler Retzlaff To: Stephen Hemminger Cc: David Marchand , dev@dpdk.org, thomas@monjalon.net, bruce.richardson@intel.com, kevin.laatz@intel.com, Parav Pandit , Xueming Li , Hemant Agrawal , Sachin Saxena , Rosen Xu , Stephen Hemminger , Long Li , Chas Williams , "Min Hu (Connor)" , Matan Azrad , Ray Kinsella , Ferruh Yigit , Andrew Rybchenko Subject: Re: [RFC PATCH 11/11] bus: hide bus object Message-ID: <20220628182335.GD17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20220628144643.1213026-1-david.marchand@redhat.com> <20220628144643.1213026-12-david.marchand@redhat.com> <20220628162213.GA17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220628092905.2da82a8c@hermes.local> <20220628170712.GC17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220628103827.5060e318@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220628103827.5060e318@hermes.local> 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 Tue, Jun 28, 2022 at 10:38:27AM -0700, Stephen Hemminger wrote: > On Tue, 28 Jun 2022 10:07:12 -0700 > Tyler Retzlaff wrote: > > > > > to avoid people tripping over mishandling pointers in/out of the api > > > > surface taking the opaque object you could declare opaque handle for the > > > > api to operate on instead. it would force the use of a cast in the > > > > implementation but would avoid accidental void * of the wrong thing that > > > > got cached being passed in. if the cast was really undesirable just > > > > whack it under an inline / internal function. > > > > > > I don't like that because it least to dangerous casts in the internal code. > > > Better to keep the the type of the object. As long as the API only passes > > > around an pointer to a struct, without exposing the contents of the struct; > > > it is safer and easier to debug. > > > > as i mentioned you can use an inline/internal function or even a macro > > to hide the cast, you could provide some additional integrity checks > > here if desired as a value add. > > > > the fact that you expose that it is a struct is an internal > > implementation detail, if truly opaque tomorrow you could convert it > > to a simple integer that indexes or enumerates something and prevents > > any meaningful interpretation in the application. > > > > when you say it is safer to debug i think you mean for dpdk devs not the > > application developer because unless the app developer does something > > really gross/dangerous casting they really can't "mishandle" the opaque > > object except to use one that isn't initialized at all which we > > can detect and handle internally in a general way. > > > > i will however concede there would be slightly more finger work when > > debugging dpdk itself since gdb / debugger doesn't automatically infer > > type so you end up having to tell gdb what is in the uintptr_t. > > > > anyway just drawing from experience in the driver frameworks we maintain > > in windows, i think one of our regrets is that we didn't do this from > > day 1 and subsequentl that we initially only used one opaque type > > instead of defining separate (not implicitly convertable) types to each > > opaque type. > > It seems to be a difference in style/taste. it's not i've sited at least one example of a mistake that becomes a compile time failure where application code is incorrectly authored where use of a pointer offers no such protection. > The Linux/Unix side prefers opaque structure pointers. > Windows (and LLVM) uses numeric handles. > > At this point DPDK should follow the Linux bus. dpdk is multi-platform and unix does not necessarily standardize on pointer to struct for opaque objects. freebsd has many apis notably bus_space that does uses handles and as previously mentioned posix threads uses handles. i understand that linux is an important platform but it isn't the only platform dpdk targets and just because it is important doesn't mean it should always enjoy being the defacto standard. anyway, i'll leave it for the patch author to decide. i still like the patch series either way. i just think this would make applications more robust.