On 11.01.2018 16:28, Tanu Kaskinen wrote:
> 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.
I have a (still unsubmitted) patch which applies on top of the
messaging/signal patches which introduces escaping. Is that
OK for you? I would submit it immediately when the other
patches are pushed. Or do you want it in this patch?
>
> 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?
Basically everything which is not enclosed in {} will be ignored.
I'll add that to the description.
(This could - if necessary - in the future be used to add a name
and a type to a field like {name.type{...} name.type{...} ...}
without breaking older parsing that does not expect such
information)
>
>> +/* 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.
>
It's still needed to check the handler name.
