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 7532FA00C5; Sat, 9 Jul 2022 10:17:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1DED54021F; Sat, 9 Jul 2022 10:17:00 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 78BA84021E for ; Sat, 9 Jul 2022 10:16:58 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1657354617; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=rzGZuqlcRYOC//TleFkBIXJoGmiwl04RQYrqXZ5km/0=; b=MZgNDc5nVxgb4MiHBJUOsvUBQJ7saR27IaxHivaINnkdIl3kFHf/FZMIHn/RioYPxf+2Qy Rdn84PEOeXtr7q163+Y05ygihdmZKAgq9R+zXKAb7umHNE+dCYnDtMpr3fmq0ubZ5JAOhJ s74MICKxdNX9EG4InBeWZ2Xuxn5GTeQ= Received: from mail-lf1-f69.google.com (mail-lf1-f69.google.com [209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-437-PbXHDwoCOmixOlgu9KJeFQ-1; Sat, 09 Jul 2022 04:16:56 -0400 X-MC-Unique: PbXHDwoCOmixOlgu9KJeFQ-1 Received: by mail-lf1-f69.google.com with SMTP id i3-20020a056512318300b0047f86b47910so391305lfe.14 for ; Sat, 09 Jul 2022 01:16:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rzGZuqlcRYOC//TleFkBIXJoGmiwl04RQYrqXZ5km/0=; b=nTHi2mCLGqTrf3v5Yw+KRRITIBzpnEfJja+nGHzfg87U4MTIVbTTXRuekgKdZdnIRg WCif7An/DFf9tCTFMTApbZAVo4AONl1WfmZdThxjsx/bqSoAqxLcj+iT3F5nDlAs7pHR ekutTdZjY4YG/voXqG4Tn62fGKi5A26SjlUq3yn37V2sZZKhg6UpFi5X7zjhsNZiFBdO TSg851asI9c4MZ8eEdSEn2Fko/Xo574n3LJB9ts4P2fPlq8MhzfhtRzysfqJQX+wm04v vcdF7m7JZKRZNk3Krx88a1YTdy1Bga5nhb+pHWM3p5+cczTP3wSuTExzQFBTRMYUB9CR 70BA== X-Gm-Message-State: AJIora9kPphauA9bMcFoKnzHnXv4BU1SuCXuCpjF0URmnbt+ZqDmxQJV cTH4BhQ2sdlhHBVJur3h8tt2vCOzVjxVsn1DZ9Ln1yOShPvWKwk5CUiPfJxdHocv3hxOHiy0tzm dACZmSVUclfuNwNDuWNw= X-Received: by 2002:ac2:593c:0:b0:489:cd83:86de with SMTP id v28-20020ac2593c000000b00489cd8386demr1159223lfi.553.1657354615081; Sat, 09 Jul 2022 01:16:55 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tFX46hHn1eLAx8FmsWcs2Xe41Z0WKzwaMNUOrg+3s7jQdT/M9GCeMSQuYMER9h+gV1oYbcFsUjDODTX/WcDVY= X-Received: by 2002:ac2:593c:0:b0:489:cd83:86de with SMTP id v28-20020ac2593c000000b00489cd8386demr1159211lfi.553.1657354614855; Sat, 09 Jul 2022 01:16:54 -0700 (PDT) MIME-Version: 1.0 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> In-Reply-To: <20220628182335.GD17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: David Marchand Date: Sat, 9 Jul 2022 10:16:43 +0200 Message-ID: Subject: Re: [RFC PATCH 11/11] bus: hide bus object To: Tyler Retzlaff Cc: Stephen Hemminger , 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 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" 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 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. -- David Marchand