Looks good. The only complaint is about the "should never happen"
handling.
--
Tanu
On Thu, 2011-10-27 at 16:45 +0200, David Henningsson wrote:
> Expose the new stuff through pacmd.
>
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
> src/pulsecore/cli-text.c | 48 +++++++++++++++++++++++++++++----------------
> 1 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/src/pulsecore/cli-text.c b/src/pulsecore/cli-text.c
> index 5498744..2253635 100644
> --- a/src/pulsecore/cli-text.c
> +++ b/src/pulsecore/cli-text.c
> @@ -102,6 +102,33 @@ char *pa_client_list_to_string(pa_core *c) {
> return pa_strbuf_tostring_free(s);
> }
>
> +static const char *port_available_to_string(pa_port_available_t a) {
> + switch (a) {
> + case PA_PORT_AVAILABLE_UNKNOWN:
> + return "unknown";
> + case PA_PORT_AVAILABLE_NO:
> + return "no";
> + case PA_PORT_AVAILABLE_YES:
> + return "yes";
> + default:
> + return "invalid"; /* Should never happen! */
As Maarten said, pa_assert_not_reached() exists for these situations.
You said that you tried to follow the surrounding convention here - I
can see that there are other to_string functions in cli-text.c that also
return something for invalid enum values. Those might be older than the
pa_assert_not_reached() function, though - to me it looks like they
could also be changed to use the assertion.
> + }
> +}
> +
> +static void append_port_list(pa_strbuf *s, pa_hashmap *ports)
> +{
> + pa_device_port *p;
> + void *state;
> +
> + if (!ports)
> + return;
> +
> + pa_strbuf_puts(s, "\tports:\n");
> + PA_HASHMAP_FOREACH(p, ports, state)
> + pa_strbuf_printf(s, "\t\t%s: %s (priority %u, available: %s)\n",
> + p->name, p->description, p->priority, port_available_to_string(p->available));
> +}
> +
> char *pa_card_list_to_string(pa_core *c) {
> pa_strbuf *s;
> pa_card *card;
> @@ -160,6 +187,8 @@ char *pa_card_list_to_string(pa_core *c) {
> for (source = pa_idxset_first(card->sources, &sidx); source; source = pa_idxset_next(card->sources, &sidx))
> pa_strbuf_printf(s, "\t\t%s/#%u: %s\n", source->name, source->index, pa_strna(pa_proplist_gets(source->proplist, PA_PROP_DEVICE_DESCRIPTION)));
> }
> +
> + append_port_list(s, card->ports);
> }
>
> return pa_strbuf_tostring_free(s);
> @@ -306,15 +335,7 @@ char *pa_sink_list_to_string(pa_core *c) {
> pa_strbuf_printf(s, "\tproperties:\n\t\t%s\n", t);
> pa_xfree(t);
>
> - if (sink->ports) {
> - pa_device_port *p;
> - void *state;
> -
> - pa_strbuf_puts(s, "\tports:\n");
> - PA_HASHMAP_FOREACH(p, sink->ports, state)
> - pa_strbuf_printf(s, "\t\t%s: %s (priority %u)\n", p->name, p->description, p->priority);
> - }
> -
> + append_port_list(s, sink->ports);
>
> if (sink->active_port)
> pa_strbuf_printf(
> @@ -429,14 +450,7 @@ char *pa_source_list_to_string(pa_core *c) {
> pa_strbuf_printf(s, "\tproperties:\n\t\t%s\n", t);
> pa_xfree(t);
>
> - if (source->ports) {
> - pa_device_port *p;
> - void *state;
> -
> - pa_strbuf_puts(s, "\tports:\n");
> - PA_HASHMAP_FOREACH(p, source->ports, state)
> - pa_strbuf_printf(s, "\t\t%s: %s (priority %u)\n", p->name, p->description, p->priority);
> - }
> + append_port_list(s, source->ports);
>
> if (source->active_port)
> pa_strbuf_printf(
