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 36D4BA0032; Sat, 9 Jul 2022 18:28:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CDAD44021F; Sat, 9 Jul 2022 18:28:08 +0200 (CEST) Received: from mail-pj1-f54.google.com (mail-pj1-f54.google.com [209.85.216.54]) by mails.dpdk.org (Postfix) with ESMTP id 35C4E4021E for ; Sat, 9 Jul 2022 18:28:07 +0200 (CEST) Received: by mail-pj1-f54.google.com with SMTP id o3-20020a17090a744300b001ef8f7f3dddso1301462pjk.3 for ; Sat, 09 Jul 2022 09:28:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=lZCKtTEWjw7rlhGnaSHGUyptsVrvBLUYzR0bUurvs94=; b=gvvmAPtPj7N6RAE7tylIss0bTNlAlnruHmfNpwqOz7HR7J5qgJ5e6ynrkAOnmkHJ4N 367DuLsYLxHZ1D6CVEtODIvCeEleRUIVE3HcWngKkMpWSA52e3uXIQouJ8VatSCsVaBx 11q+kELWRzvvxDNVzfHTWgafa/sm6GiaU80fQk0lXClDa+NcoEPfrBJKz6XzfjM3Saty DSvHa845KRX8xon1BEHPtLhY6eOCYji1dZ7gqObXGCX4ISRKsI4ULSGH0MIhRx3garlp o2e2hyd6tdEcRjph8/8i/++DnKzuJJ63p7r44qQ8RenpqSQNrc2n1p8kKfdo8XQvSaEl w9qA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=lZCKtTEWjw7rlhGnaSHGUyptsVrvBLUYzR0bUurvs94=; b=QGpNcSlnVk/Qy73O79CbBSZfdTrbis4UddYcQ05+Flh+EPUy4ej5OJM7e8vrznMgLy PK9scqZPVgBLkPoXaFxJ8VXVMJxjLptqMppOmbLBW2y4JVXc1B2gsNSKCJ9aLDcaKHYk Wdzeg+v2a4so/4po8wQG6G1uRZLpDC1tB/eGrN3wZLJVXOesaTOUH6bJkvBCLfu96s/O frmicBv64eQj6noFTmD8rNDag5Id3p1dAPTBozeUiAFzL3K4Q6jAnaReR6VNc1WaTuq7 SPVx7tHtEpjj5pUXtjWttwP0kygaVnVSG0cs6s6HSwGWuXAk7pA/JLDtcpR2gbcVNDS4 UCJw== X-Gm-Message-State: AJIora8w9dXtiGPrIkGU2Pq+YCamOz3O0zvlN5enEPTxBapKkKLZtdVf MgGBKC395s8TZ0V5iAbrpSEaUg== X-Google-Smtp-Source: AGRyM1vxvp2M6UxDjjLufhFu+3aLyO5tSqkUy/DwX/yAu3DkUFz44xYjqzjTYsJLD58i5wT1NpFa7w== X-Received: by 2002:a17:902:7042:b0:16a:2156:c325 with SMTP id h2-20020a170902704200b0016a2156c325mr9635572plt.166.1657384086421; Sat, 09 Jul 2022 09:28:06 -0700 (PDT) Received: from hermes.local (204-195-112-199.wavecable.com. [204.195.112.199]) by smtp.gmail.com with ESMTPSA id f9-20020a170902ce8900b0016bec529f77sm1513907plg.272.2022.07.09.09.28.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jul 2022 09:28:06 -0700 (PDT) Date: Sat, 9 Jul 2022 09:28:03 -0700 From: Stephen Hemminger To: David Marchand Cc: Tyler Retzlaff , dev , Thomas Monjalon , Bruce Richardson , Kevin Laatz , 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: <20220709092803.433e6644@hermes.local> In-Reply-To: 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> <20220628182335.GD17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Sat, 9 Jul 2022 10:16:43 +0200 David Marchand wrote: > On Tue, Jun 28, 2022 at 8:23 PM Tyler Retzlaff > wrote: > > > > 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. > > Thanks for this feedback Tyler. > I would lean towards Stephen opinion atm, but I am not decided yet. > > For now, I'll post a v2, extending the series to other internal objects. > We can conclude on this topic during 22.11. If you get chance to deconstruct API, switching to a numeric index is safest similar to what Tyler suggested. Think of ethdev port number and Posix file descriptor model. The advantage of an index is that it can be validated more easily by the code that is called.