[vlc-devel] [PATCH v3 1/3] Introduce media source and media tree API

[Rémi Denis-Courmont]

Le vendredi 29 juin 2018, 16:57:28 EEST Romain Vimont a écrit :
> Does it look ok to you?

Yes but it is not about me, it is about Doxygen. So check the result yourself.

> > While const is semantically correct, won't it prevent some "legitimate"
> > use of the objects from the callbacks?
> 
> I don't think so. The caller should never write to these nodes directly.

Of course, but sometimes you have things like muteces or whatever that don't 
cope with const very well, even for "reading".

> > > +} vlc_media_tree_callbacks_t;
> > 
> > I would argue that pf_ is utmostly redundant in an ops table that by
> > definition contains function pointers :/
> 
> Yes, but many of these hungarian notation prefixes are useless/redundant

Redundant is polite. They are actively harmful, alright.

Not only they waste precious columns in the source editor, but they have 
dumbed down the C type system in VLC (look at 15 years old sources and you'll 
weep).

And then, you get style horrors like this:
   foo *p_foo;
   int i_foo;
...two variables with effectively the same name.

> > Shouldn't that be merged with callback registration (what you call
> > connection, IIUC)?
> 
> I don't think so, reference counting and listeners are different things.
> Of course, adding a listener implies we should hold the object, but not
> necessarily the other way around.
> 
> > Is there a use case for referencing a tree without registering?
> 
> Currently, maybe not, but in theory it may happen (e.g. one-shot
> search in the tree).

I dunno. You should hold the lock, not a reference, when looking things up.

Otherwise you will get interesting problems when a tree is referenced by some 
GUI, but no longer by the core, and therefore no longer "findable".
 
> > Maybe expose the structure layout to registrant and make this fail-free?
> 
> It is expected to be an opaque pointer.

But registering callbacks is not generally expected to fail, and it's hardly 
ever handled correctly.

> And it would only move the problem to the caller.

Why? The caller can just embed the structure in its internal state.

-- 
Rémi Denis-Courmont