When setting attribute foo, or in this case the card profile, in my opinion the thing passed to the set_foo() function should be of the type of foo, not a string identifier that can be used to search for the actual foo in set_foo(). This is mostly a question of taste, but there's at least some small benefit from passing the actual object: often the profile object is already available when calling pa_card_set_profile(), so passing the card name would cause unnecessary searching when pa_card_set_profile() needs to look up the profile from the hashmap. --- src/modules/bluetooth/module-bluetooth-policy.c | 2 +- src/modules/bluetooth/module-bluez4-device.c | 8 ++++---- src/modules/bluetooth/module-bluez5-device.c | 8 ++++---- src/modules/dbus/iface-card-profile.c | 6 ++++++ src/modules/dbus/iface-card-profile.h | 1 + src/modules/dbus/iface-card.c | 2 +- src/modules/module-card-restore.c | 2 +- src/modules/module-switch-on-port-available.c | 2 +- src/pulsecore/card.c | 11 +++-------- src/pulsecore/card.h | 2 +- src/pulsecore/cli-command.c | 8 +++++++- src/pulsecore/protocol-native.c | 10 ++++++++-- 12 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/modules/bluetooth/module-bluetooth-policy.c b/src/modules/bluetooth/module-bluetooth-policy.c index 4a90db7..a5e9985 100644 --- a/src/modules/bluetooth/module-bluetooth-policy.c +++ b/src/modules/bluetooth/module-bluetooth-policy.c @@ -195,7 +195,7 @@ static pa_hook_result_t profile_available_hook_callback(pa_core *c, pa_card_prof pa_log_debug("Setting card '%s' to profile '%s'", card->name, selected_profile->name); - if (pa_card_set_profile(card, selected_profile->name, false) != 0) + if (pa_card_set_profile(card, selected_profile, false) != 0) pa_log_warn("Could not set profile '%s'", selected_profile->name); return PA_HOOK_OK; diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c index 2f4f71e..047332b 100644 --- a/src/modules/bluetooth/module-bluez4-device.c +++ b/src/modules/bluetooth/module-bluez4-device.c @@ -543,7 +543,7 @@ static int device_process_msg(pa_msgobject *obj, int code, void *data, int64_t o pa_log_debug("Switching the profile to off due to IO thread failure."); - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); break; } } @@ -1815,7 +1815,7 @@ static pa_hook_result_t transport_state_changed_cb(pa_bluez4_discovery *y, pa_bl pa_assert(u); if (t == u->transport && t->state == PA_BLUEZ4_TRANSPORT_STATE_DISCONNECTED) - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); if (t->device == u->device) handle_transport_state_change(u, t); @@ -2060,7 +2060,7 @@ static int card_set_profile(pa_card *c, pa_card_profile *new_profile) { off: stop_thread(u); - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); return -PA_ERR_IO; } @@ -2537,7 +2537,7 @@ int pa__init(pa_module *m) { off: stop_thread(u); - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); return 0; diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index df08d62..287e763 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -1588,7 +1588,7 @@ static int set_profile_cb(pa_card *c, pa_card_profile *new_profile) { off: stop_thread(u); - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); return -PA_ERR_IO; } @@ -1757,7 +1757,7 @@ static pa_hook_result_t transport_state_changed_cb(pa_bluetooth_discovery *y, pa pa_assert(u); if (t == u->transport && t->state <= PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED) - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); if (t->device == u->device) handle_transport_state_change(u, t); @@ -1775,7 +1775,7 @@ static int device_process_msg(pa_msgobject *obj, int code, void *data, int64_t o break; pa_log_debug("Switching the profile to off due to IO thread failure."); - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); break; } @@ -1847,7 +1847,7 @@ int pa__init(pa_module* m) { off: stop_thread(u); - pa_assert_se(pa_card_set_profile(u->card, "off", false) >= 0); + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0); return 0; diff --git a/src/modules/dbus/iface-card-profile.c b/src/modules/dbus/iface-card-profile.c index 004e2e8..2cfac7b 100644 --- a/src/modules/dbus/iface-card-profile.c +++ b/src/modules/dbus/iface-card-profile.c @@ -226,3 +226,9 @@ const char *pa_dbusiface_card_profile_get_name(pa_dbusiface_card_profile *p) { return p->profile->name; } + +pa_card_profile *pa_dbusiface_card_profile_get_profile(pa_dbusiface_card_profile *p) { + pa_assert(p); + + return p->profile; +} diff --git a/src/modules/dbus/iface-card-profile.h b/src/modules/dbus/iface-card-profile.h index 8ffb4b9..dc616d3 100644 --- a/src/modules/dbus/iface-card-profile.h +++ b/src/modules/dbus/iface-card-profile.h @@ -45,5 +45,6 @@ void pa_dbusiface_card_profile_free(pa_dbusiface_card_profile *p); const char *pa_dbusiface_card_profile_get_path(pa_dbusiface_card_profile *p); const char *pa_dbusiface_card_profile_get_name(pa_dbusiface_card_profile *p); +pa_card_profile *pa_dbusiface_card_profile_get_profile(pa_dbusiface_card_profile *p); #endif diff --git a/src/modules/dbus/iface-card.c b/src/modules/dbus/iface-card.c index da98043..b77a5e4 100644 --- a/src/modules/dbus/iface-card.c +++ b/src/modules/dbus/iface-card.c @@ -336,7 +336,7 @@ static void handle_set_active_profile(DBusConnection *conn, DBusMessage *msg, DB return; } - if ((r = pa_card_set_profile(c->card, pa_dbusiface_card_profile_get_name(new_active), true)) < 0) { + if ((r = pa_card_set_profile(c->card, pa_dbusiface_card_profile_get_profile(new_active), true)) < 0) { pa_dbus_send_error(conn, msg, DBUS_ERROR_FAILED, "Internal error in PulseAudio: pa_card_set_profile() failed with error code %i.", r); return; diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c index 19b6c25..3134067 100644 --- a/src/modules/module-card-restore.c +++ b/src/modules/module-card-restore.c @@ -413,7 +413,7 @@ static pa_hook_result_t card_profile_added_callback(pa_core *c, pa_card_profile return PA_HOOK_OK; if (pa_safe_streq(entry->profile, profile->name)) { - if (pa_card_set_profile(profile->card, profile->name, true) >= 0) + if (pa_card_set_profile(profile->card, profile, true) >= 0) pa_log_info("Restored profile '%s' for card %s.", profile->name, profile->card->name); } diff --git a/src/modules/module-switch-on-port-available.c b/src/modules/module-switch-on-port-available.c index dac599f..2c7ad17 100644 --- a/src/modules/module-switch-on-port-available.c +++ b/src/modules/module-switch-on-port-available.c @@ -121,7 +121,7 @@ static int try_to_switch_profile(pa_device_port *port) { return -1; } - if (pa_card_set_profile(port->card, best_profile->name, false) != 0) { + if (pa_card_set_profile(port->card, best_profile, false) != 0) { pa_log_debug("Could not set profile %s", best_profile->name); return -1; } diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c index 4ae16c2..eb748e6 100644 --- a/src/pulsecore/card.c +++ b/src/pulsecore/card.c @@ -255,23 +255,18 @@ void pa_card_free(pa_card *c) { pa_xfree(c); } -int pa_card_set_profile(pa_card *c, const char *name, bool save) { - pa_card_profile *profile; +int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save) { int r; pa_assert(c); + pa_assert(profile); + pa_assert(profile->card == c); if (!c->set_profile) { pa_log_debug("set_profile() operation not implemented for card %u \"%s\"", c->index, c->name); return -PA_ERR_NOTIMPLEMENTED; } - if (!name) - return -PA_ERR_NOENTITY; - - if (!(profile = pa_hashmap_get(c->profiles, name))) - return -PA_ERR_NOENTITY; - if (c->active_profile == profile) { c->save_profile = c->save_profile || save; return 0; diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h index a0f8a7d..5318150 100644 --- a/src/pulsecore/card.h +++ b/src/pulsecore/card.h @@ -115,7 +115,7 @@ void pa_card_free(pa_card *c); void pa_card_add_profile(pa_card *c, pa_card_profile *profile); -int pa_card_set_profile(pa_card *c, const char *name, bool save); +int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save); int pa_card_suspend(pa_card *c, bool suspend, pa_suspend_cause_t cause); diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c index d2cc38a..8c956ac 100644 --- a/src/pulsecore/cli-command.c +++ b/src/pulsecore/cli-command.c @@ -1634,6 +1634,7 @@ static int pa_cli_command_log_backtrace(pa_core *c, pa_tokenizer *t, pa_strbuf * static int pa_cli_command_card_profile(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) { const char *n, *p; pa_card *card; + pa_card_profile *profile; pa_core_assert_ref(c); pa_assert(t); @@ -1655,7 +1656,12 @@ static int pa_cli_command_card_profile(pa_core *c, pa_tokenizer *t, pa_strbuf *b return -1; } - if (pa_card_set_profile(card, p, true) < 0) { + if (!(profile = pa_hashmap_get(card->profiles, p))) { + pa_strbuf_printf(buf, "No such profile: %s\n", p); + return -1; + } + + if (pa_card_set_profile(card, profile, true) < 0) { pa_strbuf_printf(buf, "Failed to set card profile to '%s'.\n", p); return -1; } diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c index 21a9fe2..bf4b4a4 100644 --- a/src/pulsecore/protocol-native.c +++ b/src/pulsecore/protocol-native.c @@ -4667,8 +4667,9 @@ static void command_extension(pa_pdispatch *pd, uint32_t command, uint32_t tag, static void command_set_card_profile(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata) { pa_native_connection *c = PA_NATIVE_CONNECTION(userdata); uint32_t idx = PA_INVALID_INDEX; - const char *name = NULL, *profile = NULL; + const char *name = NULL, *profile_name = NULL; pa_card *card = NULL; + pa_card_profile *profile; int ret; pa_native_connection_assert_ref(c); @@ -4676,7 +4677,7 @@ static void command_set_card_profile(pa_pdispatch *pd, uint32_t command, uint32_ if (pa_tagstruct_getu32(t, &idx) < 0 || pa_tagstruct_gets(t, &name) < 0 || - pa_tagstruct_gets(t, &profile) < 0 || + pa_tagstruct_gets(t, &profile_name) < 0 || !pa_tagstruct_eof(t)) { protocol_error(c); return; @@ -4685,6 +4686,7 @@ static void command_set_card_profile(pa_pdispatch *pd, uint32_t command, uint32_ CHECK_VALIDITY(c->pstream, c->authorized, tag, PA_ERR_ACCESS); CHECK_VALIDITY(c->pstream, !name || pa_namereg_is_valid_name(name), tag, PA_ERR_INVALID); CHECK_VALIDITY(c->pstream, (idx != PA_INVALID_INDEX) ^ (name != NULL), tag, PA_ERR_INVALID); + CHECK_VALIDITY(c->pstream, profile_name, tag, PA_ERR_INVALID); if (idx != PA_INVALID_INDEX) card = pa_idxset_get_by_index(c->protocol->core->cards, idx); @@ -4693,6 +4695,10 @@ static void command_set_card_profile(pa_pdispatch *pd, uint32_t command, uint32_ CHECK_VALIDITY(c->pstream, card, tag, PA_ERR_NOENTITY); + profile = pa_hashmap_get(card->profiles, profile_name); + + CHECK_VALIDITY(c->pstream, profile, tag, PA_ERR_NOENTITY); + if ((ret = pa_card_set_profile(card, profile, true)) < 0) { pa_pstream_send_error(c->pstream, tag, -ret); return; -- 1.8.3.1