On Sun, 2017-10-29 at 20:51 +0100, Georg Chini wrote:
> This patch adds a small message handler to the core which enables
> clients to list available handlers via the list-handlers message.
> Command: pacmd send-message /core list-handlers
> pactl can be used with the same parameters.
>
> The patch also introduces a convention for the return string.
> It consists of a list of elements where curly braces are used
> to separate elements. Each element can itself contain further
> elements. For example consider a message that returns multiple
> elements which each contain an integer and an array of float.
> A response string would look like that:
> {{Integer} {{1st float} {2nd float} ...}}{...}
> ---
> doc/messaging_api.txt | 20 +++++++++++++------
> src/pulsecore/core.c | 43 +++++++++++++++++++++++++++++++++++++++++
> src/pulsecore/message-handler.c | 24 +++++++++++++++++++++++
> 3 files changed, 81 insertions(+), 6 deletions(-)
>
> diff --git a/doc/messaging_api.txt b/doc/messaging_api.txt
> index 11835cda..85a56d84 100644
> --- a/doc/messaging_api.txt
> +++ b/doc/messaging_api.txt
> @@ -7,10 +7,18 @@ and a message command, both specified as strings. Additional parameters can
> be specified using a single string, but are not mandatory. The message handler
> returns an error number as defined in def.h and may also return a string in
> the "response" variable. If "response" is NULL, this should be treated like
> -an empty string. The following reference lists available messages, their
> -parameters and return values.
> +an empty string. It it is not NULL, it consists of a list of elements. Curly
> +braces are used to separate elements. Each element can itself contain further
> +elements. For example consider a message that returns multiple elements which
> +each contain an integer and an array of float. A response string would look
> +like that:
> +{{Integer} {{1st float} {2nd float} ...}}{...}
As discussed earlier, escaping curly braces in strings should be
defined and supported already in the first version.
The use of whitespace needs to be defined in detail too. What
characters are considered whitespace? Is the use of whitespace
mandatory, and are there rules about the amount of whitespace?
> +/* Returns a list of handlers. */
> +static char *message_handler_list(pa_core *c) {
> + pa_strbuf *buf;
> + void *state = NULL;
> + struct pa_message_handler *handler;
> +
> + buf = pa_strbuf_new();
> +
> + while ((handler = pa_hashmap_iterate(c->message_handlers, &state, NULL))) {
PA_HASHMAP_FOREACH can be used.
> + pa_strbuf_puts(buf, "{");
> +
> + pa_strbuf_printf(buf, "{%s} {", handler->recipient);
> + if (handler->description)
> + pa_strbuf_printf(buf, "%s", handler->description);
pa_strbuf_puts() can be used here instead of pa_strbuf_printf(). We
should probably have a separate helper function for writing strings
with escaping, though.
> +/* Check if a string does not contain control characters. Currently these are
> + * only "{" and "}". */
> +static bool string_is_valid(const char *test_string) {
> + uint32_t i;
> +
> + for (i = 0; i < strlen(test_string); i++) {
This potentially calls strlen() once per character. Check
test_string[i] instead.
This function is probably not needed anyway once you add escaping
support, though.
--
Tanu
https://liberapay.com/tanuk
https://www.patreon.com/tanuk
