On Sun, 2017-10-29 at 20:51 +0100, Georg Chini wrote: > For better readability, "pactl list message-handlers" is introduced which > prints a formatted output of "pactl send-message /core list-handlers". > > The patch also adds the function pa_split_message_response() for easy > parsing of the message response string. > --- > man/pactl.1.xml.in | 2 +- > shell-completion/bash/pulseaudio | 2 +- > shell-completion/zsh/_pulseaudio | 1 + > src/pulsecore/core-util.c | 34 +++++++++++++++++++++++++++++++++ > src/pulsecore/core-util.h | 1 + > src/utils/pactl.c | 41 ++++++++++++++++++++++++++++++++++++++-- > 6 files changed, 77 insertions(+), 4 deletions(-) > > diff --git a/man/pactl.1.xml.in b/man/pactl.1.xml.in > index 9669aca9..e444f973 100644 > --- a/man/pactl.1.xml.in > +++ b/man/pactl.1.xml.in > @@ -76,7 +76,7 @@ License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>. > <option> > <p><opt>list</opt> [<arg>short</arg>] [<arg>TYPE</arg>]</p> > <optdesc><p>Dump all currently loaded modules, available sinks, sources, streams, etc. <arg>TYPE</arg> must be one of: > - modules, sinks, sources, sink-inputs, source-outputs, clients, samples, cards. If not specified, all info is listed. If > + modules, sinks, sources, sink-inputs, source-outputs, clients, samples, cards, message-handlers. If not specified, all info is listed. If > short is given, output is in a tabular format, for easy parsing by scripts.</p></optdesc> > </option> > > diff --git a/shell-completion/bash/pulseaudio b/shell-completion/bash/pulseaudio > index 797ec067..24d2aa1c 100644 > --- a/shell-completion/bash/pulseaudio > +++ b/shell-completion/bash/pulseaudio > @@ -113,7 +113,7 @@ _pactl() { > local comps > local flags='-h --help --version -s --server= --client-name=' > local list_types='short sinks sources sink-inputs source-outputs cards > - modules samples clients' > + modules samples clients message-handlers' > local commands=(stat info list exit upload-sample play-sample remove-sample > load-module unload-module move-sink-input move-source-output > suspend-sink suspend-source set-card-profile set-sink-port > diff --git a/shell-completion/zsh/_pulseaudio b/shell-completion/zsh/_pulseaudio > index a2817ebb..c24d0387 100644 > --- a/shell-completion/zsh/_pulseaudio > +++ b/shell-completion/zsh/_pulseaudio > @@ -285,6 +285,7 @@ _pactl_completion() { > 'clients: list connected clients' > 'samples: list samples' > 'cards: list available cards' > + 'message-handlers: list available message-handlers' > ) > > if ((CURRENT == 2)); then > diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c > index d4cfa20c..e7587f38 100644 > --- a/src/pulsecore/core-util.c > +++ b/src/pulsecore/core-util.c > @@ -1140,6 +1140,40 @@ const char *pa_split_spaces_in_place(const char *c, int *n, const char **state) > return current; > } > > +/* Split the specified string into elements. An element is defined as > + * a sub-string between curly braces. The function is needed to parse > + * the response of messages. Each time it is called returns a newly > + * allocated string with pa_xmalloc(). The variable state points to, > + * should be initialized to NULL before the first call. */ > +char *pa_split_message_response(const char *c, const char **state) { There are three cases where we need to parse this data format: message parameters, message responses and signal parameters. Therefore the parsing function(s) should be named in some generic manner. Clients are going to need the parsing function(s), so this should go to libpulse. I would also prefer more fine-grained functions than just one single split function. When parsing lists, this split function will unnecessarily malloc the list before mallocing the individual list elements. I already provided an example about how I'd like the parsing to work. Apparently you didn't like that for some reason? I'll copy the example here again: const char *state = message_parameters; if (pa_message_read_start_list(&state) < 0) fail("Expected a list."); while (pa_message_read_end_list(&state) < 0) { char *path; char *description; if (pa_message_read_path(&state, &path) < 0) fail("Expected a path."); if (pa_message_read_string(&state, &description) < 0) fail("Expected a string.") /* Do something with path and description. */ pa_xfree(description); pa_xfree(path); } > + const char *current = *state ? *state : c; > + const char *start_pos; > + uint32_t open_braces = 1; > + > + if (!*current || *c == 0) > + return NULL; > + > + current = strchr(current, '{'); > + if (!current) > + return NULL; > + > + start_pos = current + 1; > + I think it would be slightly easier to follow the logic if open_braces was initialized here, just before the while block. > + while (open_braces != 0 && *current != 0) { > + current++; > + if (*current == '{') > + open_braces++; > + if (*current == '}') > + open_braces--; > + } This is going to have to handle escaping. > + > + pa_assert(open_braces == 0); Invalid input shouldn't cause crashing. This will crash if the input has too few closing brackets. > + > + *state = current + 1; > + > + return pa_xstrndup(start_pos, current - start_pos); > +} > + > PA_STATIC_TLS_DECLARE(signame, pa_xfree); > > /* Return the name of an UNIX signal. Similar to Solaris sig2str() */ > diff --git a/src/pulsecore/core-util.h b/src/pulsecore/core-util.h > index e28b6aa7..947dfc13 100644 > --- a/src/pulsecore/core-util.h > +++ b/src/pulsecore/core-util.h > @@ -113,6 +113,7 @@ char *pa_split(const char *c, const char *delimiters, const char **state); > const char *pa_split_in_place(const char *c, const char *delimiters, int *n, const char **state); > char *pa_split_spaces(const char *c, const char **state); > const char *pa_split_spaces_in_place(const char *c, int *n, const char **state); > +char *pa_split_message_response(const char *c, const char **state); > > char *pa_strip_nl(char *s); > char *pa_strip(char *s); > diff --git a/src/utils/pactl.c b/src/utils/pactl.c > index 2e15b189..c7dc57ac 100644 > --- a/src/utils/pactl.c > +++ b/src/utils/pactl.c > @@ -854,6 +854,34 @@ static void string_callback(pa_context *c, int success, const char *response, vo > complete_action(); > } > > +static void list_handlers_callback(pa_context *c, int success, const char *response, void *userdata) { > + const char *state = NULL; > + char *element; > + char *handler_name; > + char *description; > + > + if (success < 0) { Just a reminder that this will have to be changed if you change the success parameter semantics as I suggested earlier. > + pa_log(_("Send message failed: %s"), pa_strerror(pa_context_errno(c))); This could be more specific, e.g. "The list-handlers message failed: %s". > + quit(1); > + return; > + } Unless short_list_format is set, there should be some heading indicating that the following list contains message handlers (the heading is useful with the plain "pactl list" command that will print many kinds of information). Or alternatively (approximately) follow the same format as with other types (I think that would be better): Message Handler /foo/bar Description: Something something something Message Handler /quux/weng Description: Coconuts are ok > + > + while ((element = pa_split_message_response(response, &state))) { > + const char *state2 = NULL; > + handler_name = pa_split_message_response(element, &state2); Error handling is missing. > + description = pa_split_message_response(element, &state2); Error handling is missing. > + if (short_list_format) > + printf("%s\n", handler_name); > + else > + printf("Name: %s Description: %s\n", handler_name, description); > + pa_xfree(element); > + pa_xfree(handler_name); > + pa_xfree(description); > + } > + > + complete_action(); > +} -- Tanu https://liberapay.com/tanuk https://www.patreon.com/tanuk
