On Tue, Jun 28, 2022 at 10:38:27AM -0700, Stephen Hemminger wrote:
> On Tue, 28 Jun 2022 10:07:12 -0700
> Tyler Retzlaff <roret...@linux.microsoft.com> 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.

Reply via email to