From: Luiz Augusto von Dentz <[email protected]>
Commit 052534ae07b8 ("transport: Update transport release flow for
bcast src") introduced a crash where it assumes transport->data always
refers to struct bap_transport which causes a crash when the transport
is in fact A2DP.
Fixes: https://github.com/bluez/bluez/issues/701
---
profiles/audio/transport.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index e2073451cc7a..0c60f06eef36 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -643,7 +643,6 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
{
struct media_transport *transport = data;
struct media_owner *owner = transport->owner;
- struct bap_transport *bap = transport->data;
const char *sender;
struct media_request *req;
guint id;
@@ -675,11 +674,6 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
req = media_request_create(msg, id);
media_owner_add(owner, req);
- if (bt_bap_stream_get_type(bap->stream) ==
- BT_BAP_STREAM_TYPE_BCAST) {
- bap_disable_complete(bap->stream, 0x00, 0x00, owner);
- }
-
return NULL;
}
@@ -1416,6 +1410,7 @@ static guint suspend_bap(struct media_transport *transport,
{
struct bap_transport *bap = transport->data;
bt_bap_stream_func_t func = NULL;
+ guint id;
if (!bap->stream)
return 0;
@@ -1427,7 +1422,14 @@ static guint suspend_bap(struct media_transport *transport,
bap_update_links(transport);
- return bt_bap_stream_disable(bap->stream, bap->linked, func, owner);
+ id = bt_bap_stream_disable(bap->stream, bap->linked, func, owner);
+
+ if (bt_bap_stream_get_type(bap->stream) == BT_BAP_STREAM_TYPE_BCAST) {
+ bap_disable_complete(bap->stream, 0x00, 0x00, owner);
+ return 0;
+ }
+
+ return id;
}
static void cancel_bap(struct media_transport *transport, guint id)
--
2.43.0
From: Luiz Augusto von Dentz <[email protected]>
This fixes the following error cause by assuming transport->data would
also be a struct bap_transport:
profiles/audio/transport.c:328:16: runtime error: load of value 2, which
is not a valid value for type '_Bool'
---
profiles/audio/transport.c | 62 ++++++++++++++++++++++++++------------
1 file changed, 43 insertions(+), 19 deletions(-)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 64b6ec694eb8..a4696154aba5 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -93,6 +93,10 @@ struct bap_transport {
struct media_transport_ops {
const char *uuid;
const GDBusPropertyTable *properties;
+ void (*set_owner)(struct media_transport *transport,
+ struct media_owner *owner);
+ void (*remove_owner)(struct media_transport *transport,
+ struct media_owner *owner);
void *(*init)(struct media_transport *transport, void *stream);
guint (*resume)(struct media_transport *transport,
struct media_owner *owner);
@@ -310,10 +314,22 @@ static guint media_transport_suspend(struct media_transport *transport,
return 0;
}
+static void transport_bap_remove_owner(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ struct bap_transport *bap = transport->data;
+
+ if (bap && bap->linked) {
+ struct bt_bap_stream *link;
+
+ link = bt_bap_stream_io_get_link(bap->stream);
+ linked_transport_remove_owner(link, owner);
+ }
+}
+
static void media_transport_remove_owner(struct media_transport *transport)
{
struct media_owner *owner = transport->owner;
- struct bap_transport *bap = transport->data;
if (!transport->owner)
return;
@@ -325,12 +341,9 @@ static void media_transport_remove_owner(struct media_transport *transport)
media_request_reply(owner->pending, EIO);
transport->owner = NULL;
- if (bap && bap->linked) {
- struct bt_bap_stream *link;
- link = bt_bap_stream_io_get_link(bap->stream);
- linked_transport_remove_owner(link, owner);
- }
+ if (transport->ops && transport->ops->remove_owner)
+ transport->ops->remove_owner(transport, owner);
if (owner->watch)
g_dbus_remove_watch(btd_get_dbus_connection(), owner->watch);
@@ -541,20 +554,27 @@ static void linked_transport_set_owner(void *data, void *user_data)
transport->owner = owner;
}
-static void media_transport_set_owner(struct media_transport *transport,
+static void transport_bap_set_owner(struct media_transport *transport,
struct media_owner *owner)
{
struct bap_transport *bap = transport->data;
- DBG("Transport %s Owner %s", transport->path, owner->name);
- transport->owner = owner;
-
- if (bap->linked) {
+ if (bap && bap->linked) {
struct bt_bap_stream *link;
link = bt_bap_stream_io_get_link(bap->stream);
linked_transport_set_owner(link, owner);
}
+}
+
+static void media_transport_set_owner(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ DBG("Transport %s Owner %s", transport->path, owner->name);
+ transport->owner = owner;
+
+ if (transport->ops && transport->ops->set_owner)
+ transport->ops->set_owner(transport, owner);
owner->transport = transport;
owner->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
@@ -1688,12 +1708,14 @@ static void *transport_bap_init(struct media_transport *transport, void *stream)
return bap;
}
-#define TRANSPORT_OPS(_uuid, _props, _init, _resume, _suspend, _cancel, \
- _set_state, _get_stream, _get_volume, _set_volume, \
- _destroy) \
+#define TRANSPORT_OPS(_uuid, _props, _set_owner, _remove_owner, _init, \
+ _resume, _suspend, _cancel, _set_state, _get_stream, \
+ _get_volume, _set_volume, _destroy) \
{ \
.uuid = _uuid, \
.properties = _props, \
+ .set_owner = _set_owner, \
+ .remove_owner = _remove_owner, \
.init = _init, \
.resume = _resume, \
.suspend = _suspend, \
@@ -1706,24 +1728,26 @@ static void *transport_bap_init(struct media_transport *transport, void *stream)
}
#define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
- TRANSPORT_OPS(_uuid, transport_a2dp_properties, _init, \
+ TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
transport_a2dp_resume, transport_a2dp_suspend, \
transport_a2dp_cancel, NULL, NULL, \
transport_a2dp_get_volume, _set_volume, \
_destroy)
-#define BAP_OPS(_uuid, _props) \
- TRANSPORT_OPS(_uuid, _props, transport_bap_init, \
+#define BAP_OPS(_uuid, _props, _set_owner, _remove_owner) \
+ TRANSPORT_OPS(_uuid, _props, _set_owner, _remove_owner,\
+ transport_bap_init, \
transport_bap_resume, transport_bap_suspend, \
transport_bap_cancel, transport_bap_set_state, \
transport_bap_get_stream, NULL, NULL, \
transport_bap_destroy)
#define BAP_UC_OPS(_uuid) \
- BAP_OPS(_uuid, transport_bap_uc_properties)
+ BAP_OPS(_uuid, transport_bap_uc_properties, \
+ transport_bap_set_owner, transport_bap_remove_owner)
#define BAP_BC_OPS(_uuid) \
- BAP_OPS(_uuid, transport_bap_bc_properties)
+ BAP_OPS(_uuid, transport_bap_bc_properties, NULL, NULL)
static struct media_transport_ops transport_ops[] = {
A2DP_OPS(A2DP_SOURCE_UUID, transport_a2dp_src_init,
--
2.43.0
From: Luiz Augusto von Dentz <[email protected]>
This creates a struct to hold the profile specific operations to avoid
having to check UUID, etc, to determine how to proceed with each
operation, it also attempts to decouple volume logic from A2DP
transports since it should be possible to support it on devices
implementing LE Audio as well.
---
profiles/audio/transport.c | 428 ++++++++++++++++++++++++-------------
profiles/audio/transport.h | 3 +-
2 files changed, 283 insertions(+), 148 deletions(-)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 0c60f06eef36..64b6ec694eb8 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -79,6 +79,7 @@ struct a2dp_transport {
struct avdtp *session;
uint16_t delay;
int8_t volume;
+ guint watch;
};
struct bap_transport {
@@ -89,6 +90,23 @@ struct bap_transport {
guint resume_id;
};
+struct media_transport_ops {
+ const char *uuid;
+ const GDBusPropertyTable *properties;
+ void *(*init)(struct media_transport *transport, void *stream);
+ guint (*resume)(struct media_transport *transport,
+ struct media_owner *owner);
+ guint (*suspend)(struct media_transport *transport,
+ struct media_owner *owner);
+ void (*cancel)(struct media_transport *transport, guint id);
+ void (*set_state)(struct media_transport *transport,
+ transport_state_t state);
+ void *(*get_stream)(struct media_transport *transport);
+ int8_t (*get_volume)(struct media_transport *transport);
+ int (*set_volume)(struct media_transport *transport, int8_t level);
+ GDestroyNotify destroy;
+};
+
struct media_transport {
char *path; /* Transport object path */
struct btd_device *device; /* Transport device */
@@ -102,20 +120,7 @@ struct media_transport {
uint16_t imtu; /* Transport input mtu */
uint16_t omtu; /* Transport output mtu */
transport_state_t state;
- guint hs_watch;
- guint source_watch;
- guint sink_watch;
- guint (*resume) (struct media_transport *transport,
- struct media_owner *owner);
- guint (*suspend) (struct media_transport *transport,
- struct media_owner *owner);
- void (*cancel) (struct media_transport *transport,
- guint id);
- void (*set_state) (struct media_transport *transport,
- transport_state_t state);
- void *(*get_stream)
- (struct media_transport *transport);
- GDestroyNotify destroy;
+ struct media_transport_ops *ops;
void *data;
};
@@ -198,20 +203,14 @@ static void transport_set_state(struct media_transport *transport,
"State");
/* Update transport specific data */
- if (transport->set_state)
- transport->set_state(transport, state);
+ if (transport->ops && transport->ops->set_state)
+ transport->ops->set_state(transport, state);
}
void media_transport_destroy(struct media_transport *transport)
{
char *path;
- if (transport->sink_watch)
- sink_remove_state_cb(transport->sink_watch);
-
- if (transport->source_watch)
- source_remove_state_cb(transport->source_watch);
-
path = g_strdup(transport->path);
g_dbus_unregister_interface(btd_get_dbus_connection(), path,
MEDIA_TRANSPORT_INTERFACE);
@@ -261,8 +260,8 @@ static void media_owner_remove(struct media_owner *owner)
DBG("Owner %s Request %s", owner->name,
dbus_message_get_member(req->msg));
- if (req->id)
- transport->cancel(transport, req->id);
+ if (req->id && transport->ops && transport->ops->cancel)
+ transport->ops->cancel(transport, req->id);
owner->pending = NULL;
if (req->msg)
@@ -297,6 +296,20 @@ static void linked_transport_remove_owner(void *data, void *user_data)
transport->owner = NULL;
}
+static guint media_transport_suspend(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ if (!state_in_use(transport->state))
+ return 0;
+
+ DBG("Transport %s Owner %s", transport->path, owner ? owner->name : "");
+
+ if (transport->ops && transport->ops->suspend)
+ return transport->ops->suspend(transport, owner);
+
+ return 0;
+}
+
static void media_transport_remove_owner(struct media_transport *transport)
{
struct media_owner *owner = transport->owner;
@@ -312,7 +325,7 @@ static void media_transport_remove_owner(struct media_transport *transport)
media_request_reply(owner->pending, EIO);
transport->owner = NULL;
- if (bap->linked) {
+ if (bap && bap->linked) {
struct bt_bap_stream *link;
link = bt_bap_stream_io_get_link(bap->stream);
@@ -324,8 +337,7 @@ static void media_transport_remove_owner(struct media_transport *transport)
media_owner_free(owner);
- if (state_in_use(transport->state))
- transport->suspend(transport, NULL);
+ media_transport_suspend(transport, NULL);
}
static gboolean media_transport_set_fd(struct media_transport *transport,
@@ -388,7 +400,7 @@ fail:
media_transport_remove_owner(transport);
}
-static guint resume_a2dp(struct media_transport *transport,
+static guint transport_a2dp_resume(struct media_transport *transport,
struct media_owner *owner)
{
struct a2dp_transport *a2dp = transport->data;
@@ -442,7 +454,7 @@ static void a2dp_suspend_complete(struct avdtp *session, int err,
media_transport_remove_owner(transport);
}
-static guint suspend_a2dp(struct media_transport *transport,
+static guint transport_a2dp_suspend(struct media_transport *transport,
struct media_owner *owner)
{
struct a2dp_transport *a2dp = transport->data;
@@ -459,11 +471,49 @@ static guint suspend_a2dp(struct media_transport *transport,
return 0;
}
-static void cancel_a2dp(struct media_transport *transport, guint id)
+static void transport_a2dp_cancel(struct media_transport *transport, guint id)
{
a2dp_cancel(id);
}
+static int8_t transport_a2dp_get_volume(struct media_transport *transport)
+{
+ struct a2dp_transport *a2dp = transport->data;
+ return a2dp->volume;
+}
+
+static int transport_a2dp_src_set_volume(struct media_transport *transport,
+ int8_t level)
+{
+ struct a2dp_transport *a2dp = transport->data;
+
+ if (a2dp->volume == level)
+ return 0;
+
+ return avrcp_set_volume(transport->device, level, false);
+}
+
+static int transport_a2dp_snk_set_volume(struct media_transport *transport,
+ int8_t level)
+{
+ struct a2dp_transport *a2dp = transport->data;
+ bool notify;
+
+ if (a2dp->volume == level)
+ return 0;
+
+ notify = a2dp->watch ? true : false;
+ if (notify) {
+ a2dp->volume = level;
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ transport->path,
+ MEDIA_TRANSPORT_INTERFACE,
+ "Volume");
+ }
+
+ return avrcp_set_volume(transport->device, level, notify);
+}
+
static void media_owner_exit(DBusConnection *connection, void *user_data)
{
struct media_owner *owner = user_data;
@@ -534,13 +584,24 @@ static void media_owner_add(struct media_owner *owner,
owner->pending = req;
}
-static void *get_stream_bap(struct media_transport *transport)
+static void *transport_bap_get_stream(struct media_transport *transport)
{
struct bap_transport *bap = transport->data;
return bap->stream;
}
+static guint media_transport_resume(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ DBG("Transport %s Owner %s", transport->path, owner ? owner->name : "");
+
+ if (transport->ops && transport->ops->resume)
+ return transport->ops->resume(transport, owner);
+
+ return 0;
+}
+
static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
void *data)
{
@@ -566,7 +627,7 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
media_transport_set_owner(transport, owner);
}
- id = transport->resume(transport, owner);
+ id = media_transport_resume(transport, owner);
if (id == 0) {
media_owner_free(owner);
return btd_error_not_authorized(msg);
@@ -599,7 +660,7 @@ static DBusMessage *try_acquire(DBusConnection *conn, DBusMessage *msg,
return btd_error_not_available(msg);
owner = media_owner_create(msg);
- id = transport->resume(transport, owner);
+ id = media_transport_resume(transport, owner);
if (id == 0) {
media_owner_free(owner);
return btd_error_not_authorized(msg);
@@ -617,8 +678,13 @@ static void bap_stop_complete(struct bt_bap_stream *stream,
void *user_data)
{
struct media_owner *owner = user_data;
- struct media_request *req = owner->pending;
- struct media_transport *transport = owner->transport;
+ struct media_request *req;
+ struct media_transport *transport;
+
+ if (!owner)
+ return;
+
+ req = owner->pending;
/* Release always succeeds */
if (req) {
@@ -627,8 +693,12 @@ static void bap_stop_complete(struct bt_bap_stream *stream,
media_owner_remove(owner);
}
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
- media_transport_remove_owner(transport);
+ transport = owner->transport;
+
+ if (transport) {
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ media_transport_remove_owner(transport);
+ }
}
static void bap_disable_complete(struct bt_bap_stream *stream,
@@ -665,7 +735,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
- id = transport->suspend(transport, owner);
+ id = media_transport_suspend(transport, owner);
if (id == 0) {
media_transport_remove_owner(transport);
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
@@ -767,32 +837,59 @@ static gboolean get_delay_reporting(const GDBusPropertyTable *property,
static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
{
struct media_transport *transport = data;
- struct a2dp_transport *a2dp = transport->data;
+ int8_t volume;
- return a2dp->volume >= 0;
+ if (media_transport_get_volume(transport, &volume))
+ return FALSE;
+
+ return volume >= 0;
+}
+
+int media_transport_get_volume(struct media_transport *transport,
+ int8_t *volume)
+{
+ if (transport->ops && transport->ops->get_volume) {
+ *volume = transport->ops->get_volume(transport);
+ return 0;
+ }
+
+ return -EINVAL;
}
static gboolean get_volume(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
struct media_transport *transport = data;
- struct a2dp_transport *a2dp = transport->data;
- uint16_t volume = (uint16_t)a2dp->volume;
+ int8_t level;
+ uint16_t volume;
+
+ if (media_transport_get_volume(transport, &level))
+ return FALSE;
+
+ volume = level;
dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
return TRUE;
}
+static int media_transport_set_volume(struct media_transport *transport,
+ int8_t level)
+{
+ DBG("Transport %s level %d", transport->path, level);
+
+ if (transport->ops && transport->ops->set_volume)
+ return transport->ops->set_volume(transport, level);
+
+ return 0;
+}
+
static void set_volume(const GDBusPropertyTable *property,
DBusMessageIter *iter, GDBusPendingPropertySet id,
void *data)
{
struct media_transport *transport = data;
- struct a2dp_transport *a2dp = transport->data;
uint16_t arg;
- int8_t volume;
- bool notify;
int err;
if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16) {
@@ -810,26 +907,13 @@ static void set_volume(const GDBusPropertyTable *property,
return;
}
- volume = (int8_t)arg;
- if (a2dp->volume == volume)
- return;
-
- notify = transport->source_watch ? true : false;
- if (notify) {
- a2dp->volume = volume;
- g_dbus_emit_property_changed(btd_get_dbus_connection(),
- transport->path,
- MEDIA_TRANSPORT_INTERFACE,
- "Volume");
- }
-
- err = avrcp_set_volume(transport->device, volume, notify);
+ err = media_transport_set_volume(transport, arg);
if (err) {
- error("avrcp_set_volume returned %s (%d)", strerror(-err), err);
+ error("Unable to set volume: %s (%d)", strerror(-err), err);
g_dbus_pending_property_error(id,
- ERROR_INTERFACE ".Failed",
- "Internal error %s (%d)",
- strerror(-err), err);
+ ERROR_INTERFACE ".Failed",
+ "Internal error %s (%d)",
+ strerror(-err), err);
return;
}
@@ -869,7 +953,7 @@ static const GDBusMethodTable transport_methods[] = {
{ },
};
-static const GDBusPropertyTable a2dp_properties[] = {
+static const GDBusPropertyTable transport_a2dp_properties[] = {
{ "Device", "o", get_device },
{ "UUID", "s", get_uuid },
{ "Codec", "y", get_codec },
@@ -1008,7 +1092,7 @@ static gboolean qos_ucast_exists(const GDBusPropertyTable *property, void *data)
return bap->qos.ucast.io_qos.phy != 0x00;
}
-static const GDBusPropertyTable bap_ucast_properties[] = {
+static const GDBusPropertyTable transport_bap_uc_properties[] = {
{ "Device", "o", get_device },
{ "UUID", "s", get_uuid },
{ "Codec", "y", get_codec },
@@ -1078,7 +1162,7 @@ static gboolean qos_bcast_exists(const GDBusPropertyTable *property, void *data)
return bap->qos.bcast.io_qos.phy != 0x00;
}
-static const GDBusPropertyTable bap_bcast_properties[] = {
+static const GDBusPropertyTable transport_bap_bc_properties[] = {
{ "Device", "o", get_device },
{ "UUID", "s", get_uuid },
{ "Codec", "y", get_codec },
@@ -1091,14 +1175,34 @@ static const GDBusPropertyTable bap_bcast_properties[] = {
{ }
};
-static void destroy_a2dp(void *data)
+static void transport_a2dp_destroy(void *data)
{
struct a2dp_transport *a2dp = data;
if (a2dp->session)
avdtp_unref(a2dp->session);
- g_free(a2dp);
+ free(a2dp);
+}
+
+static void transport_a2dp_src_destroy(void *data)
+{
+ struct a2dp_transport *a2dp = data;
+
+ if (a2dp->watch)
+ sink_remove_state_cb(a2dp->watch);
+
+ transport_a2dp_destroy(data);
+}
+
+static void transport_a2dp_snk_destroy(void *data)
+{
+ struct a2dp_transport *a2dp = data;
+
+ if (a2dp->watch)
+ source_remove_state_cb(a2dp->watch);
+
+ transport_a2dp_destroy(data);
}
static void media_transport_free(void *data)
@@ -1110,8 +1214,8 @@ static void media_transport_free(void *data)
if (transport->owner)
media_transport_remove_owner(transport);
- if (transport->destroy != NULL)
- transport->destroy(transport->data);
+ if (transport->ops && transport->ops->destroy)
+ transport->ops->destroy(transport->data);
g_free(transport->configuration);
g_free(transport->path);
@@ -1162,53 +1266,39 @@ static void source_state_changed(struct btd_service *service,
transport_update_playing(transport, FALSE);
}
-static int media_transport_init_source(struct media_transport *transport)
+static void *transport_a2dp_src_init(struct media_transport *transport,
+ void *stream)
{
struct btd_service *service;
struct a2dp_transport *a2dp;
service = btd_device_get_service(transport->device, A2DP_SINK_UUID);
- if (service == NULL)
- return -EINVAL;
-
- a2dp = g_new0(struct a2dp_transport, 1);
-
- transport->resume = resume_a2dp;
- transport->suspend = suspend_a2dp;
- transport->cancel = cancel_a2dp;
- transport->data = a2dp;
- transport->destroy = destroy_a2dp;
+ if (!service)
+ return NULL;
+ a2dp = new0(struct a2dp_transport, 1);
a2dp->volume = -1;
- transport->sink_watch = sink_add_state_cb(service, sink_state_changed,
- transport);
+ a2dp->watch = sink_add_state_cb(service, sink_state_changed, transport);
- return 0;
+ return a2dp;
}
-static int media_transport_init_sink(struct media_transport *transport)
+static void *transport_a2dp_snk_init(struct media_transport *transport,
+ void *stream)
{
struct btd_service *service;
struct a2dp_transport *a2dp;
service = btd_device_get_service(transport->device, A2DP_SOURCE_UUID);
- if (service == NULL)
- return -EINVAL;
-
- a2dp = g_new0(struct a2dp_transport, 1);
-
- transport->resume = resume_a2dp;
- transport->suspend = suspend_a2dp;
- transport->cancel = cancel_a2dp;
- transport->data = a2dp;
- transport->destroy = destroy_a2dp;
+ if (!service)
+ return NULL;
+ a2dp = new0(struct a2dp_transport, 1);
a2dp->volume = 127;
- transport->source_watch = source_add_state_cb(service,
- source_state_changed,
- transport);
+ a2dp->watch = source_add_state_cb(service, source_state_changed,
+ transport);
- return 0;
+ return a2dp;
}
static void bap_enable_complete(struct bt_bap_stream *stream,
@@ -1369,7 +1459,7 @@ static void bap_update_bcast_qos(const struct media_transport *transport)
"Configuration");
}
-static guint resume_bap(struct media_transport *transport,
+static guint transport_bap_resume(struct media_transport *transport,
struct media_owner *owner)
{
struct bap_transport *bap = transport->data;
@@ -1405,7 +1495,7 @@ static guint resume_bap(struct media_transport *transport,
return id;
}
-static guint suspend_bap(struct media_transport *transport,
+static guint transport_bap_suspend(struct media_transport *transport,
struct media_owner *owner)
{
struct bap_transport *bap = transport->data;
@@ -1432,7 +1522,7 @@ static guint suspend_bap(struct media_transport *transport,
return id;
}
-static void cancel_bap(struct media_transport *transport, guint id)
+static void transport_bap_cancel(struct media_transport *transport, guint id)
{
struct bap_transport *bap = transport->data;
@@ -1463,7 +1553,7 @@ static void link_set_state(void *data, void *user_data)
transport_set_state(transport, state);
}
-static void set_state_bap(struct media_transport *transport,
+static void transport_bap_set_state(struct media_transport *transport,
transport_state_t state)
{
struct bap_transport *bap = transport->data;
@@ -1571,7 +1661,7 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
bap_update_links(transport);
}
-static void free_bap(void *data)
+static void transport_bap_destroy(void *data)
{
struct bap_transport *bap = data;
@@ -1580,8 +1670,7 @@ static void free_bap(void *data)
free(bap);
}
-static int media_transport_init_bap(struct media_transport *transport,
- void *stream)
+static void *transport_bap_init(struct media_transport *transport, void *stream)
{
struct bt_bap_qos *qos;
struct bap_transport *bap;
@@ -1596,15 +1685,71 @@ static int media_transport_init_bap(struct media_transport *transport,
bap_connecting,
transport, NULL);
- transport->data = bap;
- transport->resume = resume_bap;
- transport->suspend = suspend_bap;
- transport->cancel = cancel_bap;
- transport->set_state = set_state_bap;
- transport->get_stream = get_stream_bap;
- transport->destroy = free_bap;
+ return bap;
+}
- return 0;
+#define TRANSPORT_OPS(_uuid, _props, _init, _resume, _suspend, _cancel, \
+ _set_state, _get_stream, _get_volume, _set_volume, \
+ _destroy) \
+{ \
+ .uuid = _uuid, \
+ .properties = _props, \
+ .init = _init, \
+ .resume = _resume, \
+ .suspend = _suspend, \
+ .cancel = _cancel, \
+ .set_state = _set_state, \
+ .get_stream = _get_stream, \
+ .get_volume = _get_volume, \
+ .set_volume = _set_volume, \
+ .destroy = _destroy \
+}
+
+#define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
+ TRANSPORT_OPS(_uuid, transport_a2dp_properties, _init, \
+ transport_a2dp_resume, transport_a2dp_suspend, \
+ transport_a2dp_cancel, NULL, NULL, \
+ transport_a2dp_get_volume, _set_volume, \
+ _destroy)
+
+#define BAP_OPS(_uuid, _props) \
+ TRANSPORT_OPS(_uuid, _props, transport_bap_init, \
+ transport_bap_resume, transport_bap_suspend, \
+ transport_bap_cancel, transport_bap_set_state, \
+ transport_bap_get_stream, NULL, NULL, \
+ transport_bap_destroy)
+
+#define BAP_UC_OPS(_uuid) \
+ BAP_OPS(_uuid, transport_bap_uc_properties)
+
+#define BAP_BC_OPS(_uuid) \
+ BAP_OPS(_uuid, transport_bap_bc_properties)
+
+static struct media_transport_ops transport_ops[] = {
+ A2DP_OPS(A2DP_SOURCE_UUID, transport_a2dp_src_init,
+ transport_a2dp_src_set_volume,
+ transport_a2dp_src_destroy),
+ A2DP_OPS(A2DP_SINK_UUID, transport_a2dp_snk_init,
+ transport_a2dp_snk_set_volume,
+ transport_a2dp_snk_destroy),
+ BAP_UC_OPS(PAC_SOURCE_UUID),
+ BAP_UC_OPS(PAC_SINK_UUID),
+ BAP_BC_OPS(BCAA_SERVICE_UUID),
+ BAP_BC_OPS(BAA_SERVICE_UUID),
+};
+
+static struct media_transport_ops *media_transport_find_ops(const char *uuid)
+{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(transport_ops); i++) {
+ struct media_transport_ops *ops = &transport_ops[i];
+
+ if (!strcasecmp(uuid, ops->uuid))
+ return ops;
+ }
+
+ return NULL;
}
struct media_transport *media_transport_create(struct btd_device *device,
@@ -1615,9 +1760,8 @@ struct media_transport *media_transport_create(struct btd_device *device,
{
struct media_endpoint *endpoint = data;
struct media_transport *transport;
- const char *uuid;
+ struct media_transport_ops *ops;
static int fd = 0;
- const GDBusPropertyTable *properties;
transport = g_new0(struct media_transport, 1);
if (device)
@@ -1642,31 +1786,21 @@ struct media_transport *media_transport_create(struct btd_device *device,
fd++);
transport->fd = -1;
- uuid = media_endpoint_get_uuid(endpoint);
- if (strcasecmp(uuid, A2DP_SOURCE_UUID) == 0) {
- if (media_transport_init_source(transport) < 0)
- goto fail;
- properties = a2dp_properties;
- } else if (strcasecmp(uuid, A2DP_SINK_UUID) == 0) {
- if (media_transport_init_sink(transport) < 0)
- goto fail;
- properties = a2dp_properties;
- } else if (!strcasecmp(uuid, PAC_SINK_UUID) ||
- !strcasecmp(uuid, PAC_SOURCE_UUID)) {
- if (media_transport_init_bap(transport, stream) < 0)
- goto fail;
- properties = bap_ucast_properties;
- } else if (!strcasecmp(uuid, BCAA_SERVICE_UUID) ||
- !strcasecmp(uuid, BAA_SERVICE_UUID)) {
- if (media_transport_init_bap(transport, stream) < 0)
- goto fail;
- properties = bap_bcast_properties;
- } else
+ ops = media_transport_find_ops(media_endpoint_get_uuid(endpoint));
+ if (!ops)
goto fail;
+ transport->ops = ops;
+
+ if (ops->init) {
+ transport->data = ops->init(transport, stream);
+ if (!transport->data)
+ goto fail;
+ }
+
if (g_dbus_register_interface(btd_get_dbus_connection(),
transport->path, MEDIA_TRANSPORT_INTERFACE,
- transport_methods, NULL, properties,
+ transport_methods, NULL, ops->properties,
transport, media_transport_free) == FALSE) {
error("Could not register transport %s", transport->path);
goto fail;
@@ -1688,8 +1822,8 @@ const char *media_transport_get_path(struct media_transport *transport)
void *media_transport_get_stream(struct media_transport *transport)
{
- if (transport->get_stream)
- return transport->get_stream(transport);
+ if (transport->ops && transport->ops->get_stream)
+ return transport->ops->get_stream(transport);
return NULL;
}
@@ -1715,12 +1849,6 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
return transport->device;
}
-int8_t media_transport_get_volume(struct media_transport *transport)
-{
- struct a2dp_transport *a2dp = transport->data;
- return a2dp->volume;
-}
-
void media_transport_update_volume(struct media_transport *transport,
int8_t volume)
{
@@ -1754,8 +1882,14 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
continue;
/* Volume is A2DP only */
- if (media_endpoint_get_sep(transport->endpoint))
- return media_transport_get_volume(transport);
+ if (media_endpoint_get_sep(transport->endpoint)) {
+ int8_t volume;
+
+ if (!media_transport_get_volume(transport, &volume))
+ return volume;
+
+ return -1;
+ }
}
/* If transport volume doesn't exists use device_volume */
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 5ca9b8f9eca9..b46bc8025418 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -21,7 +21,8 @@ void media_transport_destroy(struct media_transport *transport);
const char *media_transport_get_path(struct media_transport *transport);
void *media_transport_get_stream(struct media_transport *transport);
struct btd_device *media_transport_get_dev(struct media_transport *transport);
-int8_t media_transport_get_volume(struct media_transport *transport);
+int media_transport_get_volume(struct media_transport *transport,
+ int8_t *volume);
void media_transport_update_delay(struct media_transport *transport,
uint16_t delay);
void media_transport_update_volume(struct media_transport *transport,
--
2.43.0
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=814200
---Test result---
Test Summary:
CheckPatch FAIL 1.95 seconds
GitLint PASS 0.99 seconds
BuildEll PASS 24.60 seconds
BluezMake PASS 718.81 seconds
MakeCheck PASS 11.85 seconds
MakeDistcheck PASS 164.64 seconds
CheckValgrind PASS 226.32 seconds
CheckSmatch PASS 332.96 seconds
bluezmakeextell PASS 109.00 seconds
IncrementalBuild PASS 2001.15 seconds
ScanBuild PASS 939.70 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v2,1/3] audio: transport: Fix crash on A2DP suspend
WARNING:UNKNOWN_COMMIT_ID: Unknown commit id '052534ae07b8', maybe rebased or not pulled?
#76:
Commit 052534ae07b8 ("transport: Update transport release flow for
/github/workspace/src/src/13510505.patch total: 0 errors, 1 warnings, 40 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13510505.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[BlueZ,v2,2/3] audio/transport: Refactor transport operations
WARNING:LINE_SPACING: Missing a blank line after declarations
#253: FILE: profiles/audio/transport.c:482:
+ struct a2dp_transport *a2dp = transport->data;
+ return a2dp->volume;
/github/workspace/src/src/13510507.patch total: 0 errors, 1 warnings, 726 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13510507.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Wed, 3 Jan 2024 15:51:22 -0500 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Commit 052534ae07b8 ("transport: Update transport release flow for
> bcast src") introduced a crash where it assumes transport->data always
> refers to struct bap_transport which causes a crash when the transport
> is in fact A2DP.
>
> [...]
Here is the summary with links:
- [BlueZ,v2,1/3] audio: transport: Fix crash on A2DP suspend
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=36f057d7f66c
- [BlueZ,v2,2/3] audio/transport: Refactor transport operations
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e4764af76228
- [BlueZ,v2,3/3] audio/transport: Fix runtime error
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1c321baca781
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html