[pulseaudio-discuss] [PATCH 08/30] card: Prevent invalid profiles

Tanu Kaskinen tanu.kaskinen at linux.intel.com
Thu Jan 16 07:02:34 PST 2014


I want to add a pointer from pa_device_port to the sink/source that
the port belongs to, but if there can exist multiple sinks or sources
that have the same port, then a single pointer won't be enough. Since
it's very unlikely that we need to support multiple simultaneous sinks
or sources that contain the same port, we can just prevent such
situations from ever occurring by failing
pa_card_new_data_add_profile() if someone tries to create a profile
that would result in that kind of situation.
---
 src/modules/alsa/module-alsa-card.c             | 21 +++++++--
 src/modules/bluetooth/module-bluetooth-device.c | 21 ++++++---
 src/modules/macosx/module-coreaudio-device.c    |  7 ++-
 src/pulsecore/card.c                            | 58 ++++++++++++++++++++++++-
 src/pulsecore/card.h                            |  4 +-
 5 files changed, 97 insertions(+), 14 deletions(-)

diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
index b27f885..2601dde 100644
--- a/src/modules/alsa/module-alsa-card.c
+++ b/src/modules/alsa/module-alsa-card.c
@@ -196,11 +196,14 @@ static void add_profiles(struct userdata *u, pa_card_new_data *data) {
         d = PA_CARD_PROFILE_DATA(cp);
         d->profile = ap;
 
-        pa_card_new_data_add_profile(data, cp);
+        if (pa_card_new_data_add_profile(data, cp) < 0) {
+            pa_log("Failed to add profile %s.", cp->name);
+            pa_card_profile_free(cp);
+        }
     }
 }
 
-static void add_disabled_profile(pa_card_new_data *data) {
+static int add_disabled_profile(pa_card_new_data *data) {
     pa_card_profile *p;
     struct profile_data *d;
 
@@ -211,7 +214,13 @@ static void add_disabled_profile(pa_card_new_data *data) {
     d = PA_CARD_PROFILE_DATA(p);
     d->profile = NULL;
 
-    pa_card_new_data_add_profile(data, p);
+    if (pa_card_new_data_add_profile(data, p) < 0) {
+        pa_log("Failed to add profile %s.", p->name);
+        pa_card_profile_free(p);
+        return -1;
+    }
+
+    return 0;
 }
 
 static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
@@ -729,7 +738,11 @@ int pa__init(pa_module *m) {
         goto fail;
     }
 
-    add_disabled_profile(&data);
+    if (add_disabled_profile(&data) < 0) {
+        pa_log("Failed to add the off profile.");
+        pa_card_new_data_done(&data);
+        goto fail;
+    }
 
     if (pa_modargs_get_proplist(ma, "card_properties", data.proplist, PA_UPDATE_REPLACE) < 0) {
         pa_log("Invalid properties");
diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
index 199c85e..16c8019 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -2312,16 +2312,23 @@ static int add_card(struct userdata *u) {
             continue;
         }
 
-        pa_card_new_data_add_profile(&data, p);
+        if (pa_card_new_data_add_profile(&data, p) < 0) {
+            pa_log("Failed to add profile %s.", p->name);
+            pa_card_profile_free(p);
+        }
     }
 
-    pa_assert(!pa_hashmap_isempty(data.profiles));
-
     p = pa_card_profile_new("off", _("Off"), sizeof(enum profile));
     p->available = PA_AVAILABLE_YES;
     d = PA_CARD_PROFILE_DATA(p);
     *d = PROFILE_OFF;
-    pa_card_new_data_add_profile(&data, p);
+
+    if (pa_card_new_data_add_profile(&data, p) < 0) {
+        pa_log("Failed to add profile %s.", p->name);
+        pa_card_profile_free(p);
+        pa_card_new_data_done(&data);
+        return -1;
+    }
 
     if ((default_profile = pa_modargs_get_value(u->modargs, "profile", NULL))) {
         if (pa_hashmap_get(data.profiles, default_profile))
@@ -2421,7 +2428,11 @@ static pa_hook_result_t uuid_added_cb(pa_bluetooth_discovery *y, const struct pa
         return PA_HOOK_OK;
     }
 
-    pa_card_add_profile(u->card, p);
+    if (pa_card_add_profile(u->card, p) < 0) {
+        pa_log("Failed to add profile %s.", p->name);
+        pa_card_profile_free(p);
+        return PA_HOOK_OK;
+    }
 
     return PA_HOOK_OK;
 }
diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c
index a5e6e7b..8c81391 100644
--- a/src/modules/macosx/module-coreaudio-device.c
+++ b/src/modules/macosx/module-coreaudio-device.c
@@ -754,7 +754,12 @@ int pa__init(pa_module *m) {
 
     /* add on profile */
     p = pa_card_profile_new("on", _("On"), 0);
-    pa_card_new_data_add_profile(&card_new_data, p);
+
+    if (pa_card_new_data_add_profile(&card_new_data, p) < 0) {
+        pa_log("Failed to add profile %s.", p->name);
+        pa_card_new_data_done(&card_new_data);
+        goto fail;
+    }
 
     /* create the card object */
     u->card = pa_card_new(m->core, &card_new_data);
diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
index b780c5b..c21803e 100644
--- a/src/pulsecore/card.c
+++ b/src/pulsecore/card.c
@@ -135,6 +135,50 @@ void pa_card_new_data_set_name(pa_card_new_data *data, const char *name) {
     data->name = pa_xstrdup(name);
 }
 
+static bool profile_valid(pa_card_profile *profile) {
+    pa_device_prototype *prototype, *prototype2;
+    pa_device_port *port;
+    void *state, *state2, *state3;
+
+    pa_assert(profile);
+
+    /* A profile must not have ports that are part of multiple devices on that
+     * profile. While this limitation isn't something that absolutely must
+     * exist, the alternative would be to have lots of complexity for handling
+     * cases where a profile has multiple devices that can activate the same
+     * port. */
+
+    PA_HASHMAP_FOREACH(prototype, profile->sink_prototypes, state) {
+        PA_HASHMAP_FOREACH(port, prototype->ports, state2) {
+            PA_HASHMAP_FOREACH(prototype2, profile->sink_prototypes, state3) {
+                if (prototype2 == prototype)
+                    continue;
+
+                if (pa_hashmap_get(prototype2->ports, port->name)) {
+                    pa_log("Port %s is part of multiple devices on profile %s.", port->name, profile->name);
+                    return false;
+                }
+            }
+        }
+    }
+
+    PA_HASHMAP_FOREACH(prototype, profile->source_prototypes, state) {
+        PA_HASHMAP_FOREACH(port, prototype->ports, state2) {
+            PA_HASHMAP_FOREACH(prototype2, profile->source_prototypes, state3) {
+                if (prototype2 == prototype)
+                    continue;
+
+                if (pa_hashmap_get(prototype2->ports, port->name)) {
+                    pa_log("Port %s is part of multiple devices on profile %s.", port->name, profile->name);
+                    return false;
+                }
+            }
+        }
+    }
+
+    return true;
+}
+
 void pa_card_new_data_add_device_prototype(pa_card_new_data *data, pa_device_prototype *prototype) {
     pa_assert(data);
     pa_assert(prototype);
@@ -142,7 +186,7 @@ void pa_card_new_data_add_device_prototype(pa_card_new_data *data, pa_device_pro
     pa_hashmap_put(data->device_prototypes, prototype, prototype);
 }
 
-void pa_card_new_data_add_profile(pa_card_new_data *data, pa_card_profile *profile) {
+int pa_card_new_data_add_profile(pa_card_new_data *data, pa_card_profile *profile) {
     pa_device_prototype *prototype;
     pa_device_port *port;
     void *state, *state2;
@@ -150,12 +194,17 @@ void pa_card_new_data_add_profile(pa_card_new_data *data, pa_card_profile *profi
     pa_assert(data);
     pa_assert(profile);
 
+    if (!profile_valid(profile))
+        return -PA_ERR_INVALID;
+
     pa_assert_se(pa_hashmap_put(data->profiles, profile->name, profile) >= 0);
 
     PA_HASHMAP_FOREACH(prototype, profile->sink_prototypes, state) {
         PA_HASHMAP_FOREACH(port, prototype->ports, state2)
             pa_hashmap_put(port->profiles, profile->name, profile);
     }
+
+    return 0;
 }
 
 void pa_card_new_data_set_profile(pa_card_new_data *data, const char *profile) {
@@ -319,7 +368,7 @@ void pa_card_free(pa_card *c) {
     pa_xfree(c);
 }
 
-void pa_card_add_profile(pa_card *c, pa_card_profile *profile) {
+int pa_card_add_profile(pa_card *c, pa_card_profile *profile) {
     pa_device_prototype *prototype;
     pa_device_port *port;
     void *state, *state2;
@@ -327,6 +376,9 @@ void pa_card_add_profile(pa_card *c, pa_card_profile *profile) {
     pa_assert(c);
     pa_assert(profile);
 
+    if (!profile_valid(profile))
+        return -PA_ERR_INVALID;
+
     /* take ownership of the profile */
     pa_assert_se(pa_hashmap_put(c->profiles, profile->name, profile) >= 0);
     profile->card = c;
@@ -339,6 +391,8 @@ void pa_card_add_profile(pa_card *c, pa_card_profile *profile) {
     pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index);
 
     pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_ADDED], profile);
+
+    return 0;
 }
 
 int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save, bool bypass_router) {
diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h
index 4e89302..83884a6 100644
--- a/src/pulsecore/card.h
+++ b/src/pulsecore/card.h
@@ -156,7 +156,7 @@ void pa_card_new_data_set_name(pa_card_new_data *data, const char *name);
 void pa_card_new_data_add_device_prototype(pa_card_new_data *data, pa_device_prototype *prototype);
 
 /* The ownership of the profile moves to pa_card_new_data. */
-void pa_card_new_data_add_profile(pa_card_new_data *data, pa_card_profile *profile);
+int pa_card_new_data_add_profile(pa_card_new_data *data, pa_card_profile *profile);
 
 void pa_card_new_data_set_profile(pa_card_new_data *data, const char *profile);
 void pa_card_new_data_set_recreate_devices_on_profile_switch(pa_card_new_data *data, bool recreate);
@@ -166,7 +166,7 @@ pa_card *pa_card_new(pa_core *c, pa_card_new_data *data);
 void pa_card_free(pa_card *c);
 
 /* The ownership of the profile moves to pa_card. */
-void pa_card_add_profile(pa_card *c, pa_card_profile *profile);
+int pa_card_add_profile(pa_card *c, pa_card_profile *profile);
 
 int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save, bool bypass_router);
 
-- 
1.8.3.1



More information about the pulseaudio-discuss mailing list