On 02.10.2017 11:31, Tanu Kaskinen wrote:
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).
You want a header file just for this struct? I would keep it in
core-messages.h but if you prefer, I don't mind using a
separate file.
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.
If the struct is made public, the function is simply obsolete.
The core contains a pointer to the hashmap. Formatting the
string can then be done directly in the message handler.
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?
I don't understand. Message handlers return a single string, so
parsing the result is the task of the sender. The format returned
by the handler is rather simple, apart from the header line
it only consists of lines of the form
handler-path, " ", description
This should be easy to parse.
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.
Again I don't understand, see above. list-handlers is a message
sent to the core, not a command. As such, it returns a string.
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?
Two reasons:
a) It's much simpler than using the introspection API
b) We should at least have one example in the code
which shows how a message handler i implemented.
And one more thing: if the handler descriptions are visible to clients
and the descriptions are mutable, clients should be notified about
description changes.
How? Sending a signal?
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
