[pulseaudio-discuss] [PATCH 4/6] core: add message handler

[Tanu Kaskinen]

On Sun, 2017-10-01 at 20:31 +0200, Georg Chini wrote:
> On 01.10.2017 18:16, Tanu Kaskinen wrote:
> > On Sat, 2017-08-19 at 17:48 +0200, Georg Chini wrote:
> > > +/* List handlers */
> > > +char *pa_core_message_handler_list(pa_core *c);
> > 
> > Putting this function to core-messages.h doesn't seem right to me. The
> > function will never be used outside core.c, so the it should be a
> > private function in core.c.
> > 
> 
> If I put it in core.c, I would need to expose the pa_core_message_handler
> structure, which you wanted to keep private.

I think it actually makes sense to make the struct public. It could be
private if it didn't have the description field, but since the handler
objects contain information that clients can query, they're similar to
all other core objects. I think it even makes sense to move the struct
to a separate message-handler.h header (but if you don't like that
idea, I'm fine with keeping it in core-messaging.h or moving it to
core.h).

> Also I am not sure if the 
> function
> will never be called from outside core.c. It might be useful to know,
> which message handlers are present.

pa_core_message_handler_list() returns a string whose format doesn't
have a specification, so it's not possible to reliably get the message
handler information that way. And parsing the string would be more
cumbersome anyway than having the information directly accessible via
the pa_core_message_handler struct if that's made public.

Something that didn't occur to me when I reviewed this patch yesterday
is that the string that the list-handlers returns is tailor-made for
pactl/pacmd, which is not how the message handlers are expected to
behave. What if an application wants to not only print a message (of
which layout is already decided by the server), but to get a list of
handlers and do something else with that information? The list-handlers 
command should return machine-readable information: either only a list
of the object paths (with the possibility to query the description of a
handler with another message), or a dictionary whose keys are object
paths and values are dictionaries representing the handler objects. The
per-object dictionary would contain property names as keys (currently
the description is the only property) and property values as variant
values (by variant I mean the D-Bus definition of the word). Returning
a list containing plain (path, description) pairs wouldn't be a good
idea, because that would cause trouble when adding new properties to
the handler objects.

Also, it's questionable why the list-handlers command is implemented
with the messaging API in the first place. It's core functionality, so
why is it not implemented using the normal introspection API?

And one more thing: if the handler descriptions are visible to clients
and the descriptions are mutable, clients should be notified about
description changes.

-- 
Tanu

https://www.patreon.com/tanuk