[pulseaudio-discuss] [PATCH] bluetooth: don't create the HSP/HFP profile twice

Tanu Kaskinen tanuk at iki.fi
Sat Jul 30 23:44:55 UTC 2016


create_card_profile() used to get called separately for HSP and HFP,
so if a headset supports both profiles, a profile named
"headset_head_unit" would get created twice. The second instance would
get immediately freed, so that wasn't a particularly serious problem.
However, I think it makes more sense to create the profile only once.
This patch makes things so that before a profile is created, we check
what name that profile would have, and if a profile with that name
already exists, we don't create the profile.

A couple of Yocto releases (jethro and krogoth) have non-upstream
patches that suffer from this double creation. The patches add
associations between profiles and ports, and those associations use
the profile name as the key. When the second profile gets freed, the
associations between the profile and its ports get removed, and since
the profile name is used as the key, this erroneously affects the
first profile too. Crashing ensues.

I have tested this only with BlueZ 5.

BugLink: https://bugzilla.yoctoproject.org/show_bug.cgi?id=10018
---
 src/modules/bluetooth/module-bluez4-device.c | 81 +++++++++++++++++-----------
 src/modules/bluetooth/module-bluez5-device.c | 72 ++++++++++++++++---------
 2 files changed, 99 insertions(+), 54 deletions(-)

diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c
index 13fb7ab..be29a73 100644
--- a/src/modules/bluetooth/module-bluez4-device.c
+++ b/src/modules/bluetooth/module-bluez4-device.c
@@ -2162,18 +2162,23 @@ static void create_card_ports(struct userdata *u, pa_hashmap *ports) {
 }
 
 /* Run from main thread */
-static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid, pa_hashmap *ports) {
+static pa_card_profile *create_card_profile(struct userdata *u, pa_bluez4_profile_t profile, pa_hashmap *ports) {
     pa_device_port *input_port, *output_port;
+    const char *name;
     pa_card_profile *p = NULL;
-    pa_bluez4_profile_t *d;
+    pa_bluez4_profile_t *d = NULL;
+    pa_bluez4_transport *t;
 
     pa_assert(u->input_port_name);
     pa_assert(u->output_port_name);
     pa_assert_se(input_port = pa_hashmap_get(ports, u->input_port_name));
     pa_assert_se(output_port = pa_hashmap_get(ports, u->output_port_name));
 
-    if (pa_streq(uuid, A2DP_SINK_UUID)) {
-        p = pa_card_profile_new("a2dp", _("High Fidelity Playback (A2DP)"), sizeof(pa_bluez4_profile_t));
+    name = pa_bluez4_profile_to_string(profile);
+
+    switch (profile) {
+    case PA_BLUEZ4_PROFILE_A2DP:
+        p = pa_card_profile_new(name, _("High Fidelity Playback (A2DP)"), sizeof(pa_bluez4_profile_t));
         p->priority = 10;
         p->n_sinks = 1;
         p->n_sources = 0;
@@ -2182,9 +2187,10 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(output_port->profiles, p->name, p);
 
         d = PA_CARD_PROFILE_DATA(p);
-        *d = PA_BLUEZ4_PROFILE_A2DP;
-    } else if (pa_streq(uuid, A2DP_SOURCE_UUID)) {
-        p = pa_card_profile_new("a2dp_source", _("High Fidelity Capture (A2DP)"), sizeof(pa_bluez4_profile_t));
+        break;
+
+    case PA_BLUEZ4_PROFILE_A2DP_SOURCE:
+        p = pa_card_profile_new(name, _("High Fidelity Capture (A2DP)"), sizeof(pa_bluez4_profile_t));
         p->priority = 10;
         p->n_sinks = 0;
         p->n_sources = 1;
@@ -2193,9 +2199,10 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(input_port->profiles, p->name, p);
 
         d = PA_CARD_PROFILE_DATA(p);
-        *d = PA_BLUEZ4_PROFILE_A2DP_SOURCE;
-    } else if (pa_streq(uuid, HSP_HS_UUID) || pa_streq(uuid, HFP_HS_UUID)) {
-        p = pa_card_profile_new("hsp", _("Telephony Duplex (HSP/HFP)"), sizeof(pa_bluez4_profile_t));
+        break;
+
+    case PA_BLUEZ4_PROFILE_HSP:
+        p = pa_card_profile_new(name, _("Telephony Duplex (HSP/HFP)"), sizeof(pa_bluez4_profile_t));
         p->priority = 20;
         p->n_sinks = 1;
         p->n_sources = 1;
@@ -2205,9 +2212,10 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(output_port->profiles, p->name, p);
 
         d = PA_CARD_PROFILE_DATA(p);
-        *d = PA_BLUEZ4_PROFILE_HSP;
-    } else if (pa_streq(uuid, HFP_AG_UUID)) {
-        p = pa_card_profile_new("hfgw", _("Handsfree Gateway"), sizeof(pa_bluez4_profile_t));
+        break;
+
+    case PA_BLUEZ4_PROFILE_HFGW:
+        p = pa_card_profile_new(name, _("Handsfree Gateway"), sizeof(pa_bluez4_profile_t));
         p->priority = 20;
         p->n_sinks = 1;
         p->n_sources = 1;
@@ -2217,19 +2225,35 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(output_port->profiles, p->name, p);
 
         d = PA_CARD_PROFILE_DATA(p);
-        *d = PA_BLUEZ4_PROFILE_HFGW;
+        break;
+
+    case PA_BLUEZ4_PROFILE_OFF:
+        pa_assert_not_reached();
     }
 
-    if (p) {
-        pa_bluez4_transport *t;
+    *d = profile;
 
-        if ((t = u->device->transports[*d]))
-            p->available = transport_state_to_availability(t->state);
-    }
+    if ((t = u->device->transports[*d]))
+        p->available = transport_state_to_availability(t->state);
 
     return p;
 }
 
+static int uuid_to_profile(const char *uuid, pa_bluez4_profile_t *_r) {
+    if (pa_streq(uuid, A2DP_SINK_UUID))
+        *_r = PA_BLUEZ4_PROFILE_A2DP;
+    else if (pa_streq(uuid, A2DP_SOURCE_UUID))
+        *_r = PA_BLUEZ4_PROFILE_A2DP_SOURCE;
+    else if (pa_streq(uuid, HSP_HS_UUID) || pa_streq(uuid, HFP_HS_UUID))
+        *_r = PA_BLUEZ4_PROFILE_HSP;
+    else if (pa_streq(uuid, HSP_AG_UUID) || pa_streq(uuid, HFP_AG_UUID))
+        *_r = PA_BLUEZ4_PROFILE_HFGW;
+    else
+        return -PA_ERR_INVALID;
+
+    return 0;
+}
+
 /* Run from main thread */
 static int add_card(struct userdata *u) {
     pa_card_new_data data;
@@ -2277,16 +2301,15 @@ static int add_card(struct userdata *u) {
     create_card_ports(u, data.ports);
 
     PA_LLIST_FOREACH(uuid, device->uuids) {
-        p = create_card_profile(u, uuid->uuid, data.ports);
+        pa_bluez4_profile_t profile;
 
-        if (!p)
+        if (uuid_to_profile(uuid->uuid, &profile) < 0)
             continue;
 
-        if (pa_hashmap_get(data.profiles, p->name)) {
-            pa_card_profile_free(p);
+        if (pa_hashmap_get(data.profiles, pa_bluez4_profile_to_string(profile)))
             continue;
-        }
 
+        p = create_card_profile(u, profile, data.ports);
         pa_hashmap_put(data.profiles, p->name, p);
     }
 
@@ -2386,6 +2409,7 @@ static pa_bluez4_device* find_device(struct userdata *u, const char *address, co
 /* Run from main thread */
 static pa_hook_result_t uuid_added_cb(pa_bluez4_discovery *y, const struct pa_bluez4_hook_uuid_data *data,
                                       struct userdata *u) {
+    pa_bluez4_profile_t profile;
     pa_card_profile *p;
 
     pa_assert(data);
@@ -2396,16 +2420,13 @@ static pa_hook_result_t uuid_added_cb(pa_bluez4_discovery *y, const struct pa_bl
     if (data->device != u->device)
         return PA_HOOK_OK;
 
-    p = create_card_profile(u, data->uuid, u->card->ports);
-
-    if (!p)
+    if (uuid_to_profile(data->uuid, &profile) < 0)
         return PA_HOOK_OK;
 
-    if (pa_hashmap_get(u->card->profiles, p->name)) {
-        pa_card_profile_free(p);
+    if (pa_hashmap_get(u->card->profiles, pa_bluez4_profile_to_string(profile)))
         return PA_HOOK_OK;
-    }
 
+    p = create_card_profile(u, profile, u->card->ports);
     pa_card_add_profile(u->card, p);
 
     return PA_HOOK_OK;
diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index 498d0e1..e610095 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -1772,8 +1772,9 @@ static void create_card_ports(struct userdata *u, pa_hashmap *ports) {
 }
 
 /* Run from main thread */
-static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid, pa_hashmap *ports) {
+static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_profile_t profile, pa_hashmap *ports) {
     pa_device_port *input_port, *output_port;
+    const char *name;
     pa_card_profile *cp = NULL;
     pa_bluetooth_profile_t *p;
 
@@ -1782,8 +1783,11 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
     pa_assert_se(input_port = pa_hashmap_get(ports, u->input_port_name));
     pa_assert_se(output_port = pa_hashmap_get(ports, u->output_port_name));
 
-    if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK)) {
-        cp = pa_card_profile_new("a2dp_sink", _("High Fidelity Playback (A2DP Sink)"), sizeof(pa_bluetooth_profile_t));
+    name = pa_bluetooth_profile_to_string(profile);
+
+    switch (profile) {
+    case PA_BLUETOOTH_PROFILE_A2DP_SINK:
+        cp = pa_card_profile_new(name, _("High Fidelity Playback (A2DP Sink)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 10;
         cp->n_sinks = 1;
         cp->n_sources = 0;
@@ -1792,9 +1796,10 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(output_port->profiles, cp->name, cp);
 
         p = PA_CARD_PROFILE_DATA(cp);
-        *p = PA_BLUETOOTH_PROFILE_A2DP_SINK;
-    } else if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE)) {
-        cp = pa_card_profile_new("a2dp_source", _("High Fidelity Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
+        break;
+
+    case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
+        cp = pa_card_profile_new(name, _("High Fidelity Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 10;
         cp->n_sinks = 0;
         cp->n_sources = 1;
@@ -1803,9 +1808,10 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(input_port->profiles, cp->name, cp);
 
         p = PA_CARD_PROFILE_DATA(cp);
-        *p = PA_BLUETOOTH_PROFILE_A2DP_SOURCE;
-    } else if (pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_HS) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF)) {
-        cp = pa_card_profile_new("headset_head_unit", _("Headset Head Unit (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
+        break;
+
+    case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+        cp = pa_card_profile_new(name, _("Headset Head Unit (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 20;
         cp->n_sinks = 1;
         cp->n_sources = 1;
@@ -1815,9 +1821,10 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(output_port->profiles, cp->name, cp);
 
         p = PA_CARD_PROFILE_DATA(cp);
-        *p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
-    } else if (pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_AG) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_AG)) {
-        cp = pa_card_profile_new("headset_audio_gateway", _("Headset Audio Gateway (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
+        break;
+
+    case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+        cp = pa_card_profile_new(name, _("Headset Audio Gateway (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 20;
         cp->n_sinks = 1;
         cp->n_sources = 1;
@@ -1827,16 +1834,19 @@ static pa_card_profile *create_card_profile(struct userdata *u, const char *uuid
         pa_hashmap_put(output_port->profiles, cp->name, cp);
 
         p = PA_CARD_PROFILE_DATA(cp);
-        *p = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
-    }
+        break;
 
-    if (cp) {
-        if (u->device->transports[*p])
-            cp->available = transport_state_to_availability(u->device->transports[*p]->state);
-        else
-            cp->available = PA_AVAILABLE_NO;
+    case PA_BLUETOOTH_PROFILE_OFF:
+        pa_assert_not_reached();
     }
 
+    *p = profile;
+
+    if (u->device->transports[*p])
+        cp->available = transport_state_to_availability(u->device->transports[*p]->state);
+    else
+        cp->available = PA_AVAILABLE_NO;
+
     return cp;
 }
 
@@ -1882,6 +1892,21 @@ off:
     return -PA_ERR_IO;
 }
 
+static int uuid_to_profile(const char *uuid, pa_bluetooth_profile_t *_r) {
+    if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
+        *_r = PA_BLUETOOTH_PROFILE_A2DP_SINK;
+    else if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
+        *_r = PA_BLUETOOTH_PROFILE_A2DP_SOURCE;
+    else if (pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_HS) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF))
+        *_r = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+    else if (pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_AG) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_AG))
+        *_r = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
+    else
+        return -PA_ERR_INVALID;
+
+    return 0;
+}
+
 /* Run from main thread */
 static int add_card(struct userdata *u) {
     const pa_bluetooth_device *d;
@@ -1923,16 +1948,15 @@ static int add_card(struct userdata *u) {
     create_card_ports(u, data.ports);
 
     PA_HASHMAP_FOREACH(uuid, d->uuids, state) {
-        cp = create_card_profile(u, uuid, data.ports);
+        pa_bluetooth_profile_t profile;
 
-        if (!cp)
+        if (uuid_to_profile(uuid, &profile) < 0)
             continue;
 
-        if (pa_hashmap_get(data.profiles, cp->name)) {
-            pa_card_profile_free(cp);
+        if (pa_hashmap_get(data.profiles, pa_bluetooth_profile_to_string(profile)))
             continue;
-        }
 
+        cp = create_card_profile(u, profile, data.ports);
         pa_hashmap_put(data.profiles, cp->name, cp);
     }
 
-- 
2.8.1



More information about the pulseaudio-discuss mailing list