This patch implements a BASS fix for adding watches for closed
channels, adds btio support for binding a PA sync channel to
a number of BISes and adds an initial implementation for the
BASS Modify Source procedure.
This patch series depends on the kernel support added by
Bluetooth: ISO: Allow binding a PA sync socket
Iulia Tanasescu (4):
btio: Handle closed channel in server_cb
shared/bass: Handle G_IO_HUP on io channels
btio: Allow binding a bcast listener before accept
shared/bass: Add Modify Source initial implementation
btio/btio.c | 54 ++++++-
btio/btio.h | 2 +-
profiles/audio/bap.c | 2 +-
src/shared/bass.c | 333 ++++++++++++++++++++++++++++++++++++++++---
src/shared/bass.h | 4 +
5 files changed, 371 insertions(+), 24 deletions(-)
base-commit: e347792f41a8ab2cf4f789b6544a4f8ec8464a53
--
2.39.2
This handles G_IO_ERR and G_IO_HUP conditions in server_cb
---
btio/btio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/btio/btio.c b/btio/btio.c
index d45b8240d..c63a6d1df 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io, GIOCondition cond,
GIOChannel *cli_io;
/* If the user closed the server */
- if ((cond & G_IO_NVAL) || check_nval(io))
+ if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
+ check_nval(io))
return FALSE;
srv_sock = g_io_channel_unix_get_fd(io);
--
2.39.2
This adds watches to handle closed sockets
---
src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
src/shared/bass.h | 2 ++
2 files changed, 61 insertions(+)
diff --git a/src/shared/bass.c b/src/shared/bass.c
index 0ee3187d1..78103d463 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
g_io_channel_unref(bcast_src->listen_io);
bcast_src->listen_io = NULL;
+ if (bcast_src->listen_io_id > 0) {
+ g_source_remove(bcast_src->listen_io_id);
+ bcast_src->listen_io_id = 0;
+ }
+
/* Close pa sync io */
if (bcast_src->pa_sync_io) {
g_io_channel_shutdown(bcast_src->pa_sync_io,
@@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
bcast_src->pa_sync_io = NULL;
}
+ if (bcast_src->pa_sync_io_id > 0) {
+ g_source_remove(bcast_src->pa_sync_io_id);
+ bcast_src->pa_sync_io_id = 0;
+ }
+
for (i = 0; i < bcast_src->num_subgroups; i++)
bcast_src->subgroup_data[i].bis_sync =
BT_BASS_BIG_SYNC_FAILED_BITMASK;
@@ -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src *bcast_src)
return false;
}
+static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
+ gpointer data)
+{
+ struct bt_bcast_src *bcast_src = data;
+
+ DBG(bcast_src->bass, "PA sync io has been disconnected");
+
+ bcast_src->pa_sync_io_id = 0;
+ bcast_src->pa_sync_io = NULL;
+
+ return FALSE;
+}
static void confirm_cb(GIOChannel *io, gpointer user_data)
{
@@ -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
bcast_src->pa_sync_io = io;
g_io_channel_ref(bcast_src->pa_sync_io);
+ if (bcast_src->pa_sync_io_id > 0) {
+ g_source_remove(bcast_src->pa_sync_io_id);
+ bcast_src->pa_sync_io_id = 0;
+ }
+
+ bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR | G_IO_HUP |
+ G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
+ bcast_src);
+
len = sizeof(qos);
memset(&qos, 0, len);
@@ -844,6 +875,19 @@ static bool bass_validate_add_src_params(uint8_t *value, size_t len)
return true;
}
+static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
+ gpointer data)
+{
+ struct bt_bcast_src *bcast_src = data;
+
+ DBG(bcast_src->bass, "Listen io has been disconnected");
+
+ bcast_src->listen_io_id = 0;
+ bcast_src->listen_io = NULL;
+
+ return FALSE;
+}
+
static void bass_handle_add_src_op(struct bt_bass *bass,
struct gatt_db_attribute *attrib,
uint8_t opcode,
@@ -1023,6 +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
bcast_src->listen_io = io;
g_io_channel_ref(bcast_src->listen_io);
+ bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
+ G_IO_HUP | G_IO_NVAL,
+ (GIOFunc)listen_io_disconnect_cb,
+ bcast_src);
+
if (num_bis > 0 && !bcast_src->bises)
bcast_src->bises = queue_new();
} else {
@@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
g_io_channel_unref(bcast_src->listen_io);
}
+ if (bcast_src->listen_io_id > 0) {
+ g_source_remove(bcast_src->listen_io_id);
+ bcast_src->listen_io_id = 0;
+ }
+
if (bcast_src->pa_sync_io) {
g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
g_io_channel_unref(bcast_src->pa_sync_io);
}
+ if (bcast_src->pa_sync_io_id > 0) {
+ g_source_remove(bcast_src->pa_sync_io_id);
+ bcast_src->pa_sync_io_id = 0;
+ }
+
queue_destroy(bcast_src->bises, bass_bis_unref);
free(bcast_src);
diff --git a/src/shared/bass.h b/src/shared/bass.h
index c4b5b76ba..bd3fe900b 100644
--- a/src/shared/bass.h
+++ b/src/shared/bass.h
@@ -57,7 +57,9 @@ struct bt_bcast_src {
uint8_t num_subgroups;
struct bt_bass_subgroup_data *subgroup_data;
GIOChannel *listen_io;
+ guint listen_io_id;
GIOChannel *pa_sync_io;
+ guint pa_sync_io_id;
struct queue *bises;
};
--
2.39.2
This adds btio support for binding a PA sync io to a number of BISes,
before proceeding with BIG Create Sync
---
btio/btio.c | 51 +++++++++++++++++++++++++++++++++++++++++++-
btio/btio.h | 2 +-
profiles/audio/bap.c | 2 +-
src/shared/bass.c | 9 ++++----
4 files changed, 57 insertions(+), 7 deletions(-)
diff --git a/btio/btio.c b/btio/btio.c
index c63a6d1df..338f24916 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -1797,14 +1797,63 @@ gboolean bt_io_accept(GIOChannel *io, BtIOConnect connect, gpointer user_data,
gboolean bt_io_bcast_accept(GIOChannel *io, BtIOConnect connect,
gpointer user_data, GDestroyNotify destroy,
- GError * *err)
+ GError * *err, BtIOOption opt1, ...)
{
int sock;
char c;
struct pollfd pfd;
+ va_list args;
+ struct sockaddr_iso *addr = NULL;
+ uint8_t bc_num_bis = 0;
+ uint8_t bc_bis[ISO_MAX_NUM_BIS] = {0};
+ BtIOOption opt = opt1;
+
+ va_start(args, opt1);
+
+ while (opt != BT_IO_OPT_INVALID) {
+ if (opt == BT_IO_OPT_ISO_BC_NUM_BIS) {
+ bc_num_bis = va_arg(args, int);
+ } else if (opt == BT_IO_OPT_ISO_BC_BIS) {
+ memcpy(bc_bis, va_arg(args, uint8_t *),
+ bc_num_bis);
+ } else {
+ g_set_error(err, BT_IO_ERROR, EINVAL,
+ "Invalid option %d", opt);
+ break;
+ }
+
+ opt = va_arg(args, int);
+ }
+
+ va_end(args);
+
+ if (*err)
+ return FALSE;
sock = g_io_channel_unix_get_fd(io);
+ if (bc_num_bis) {
+ addr = malloc(sizeof(*addr) + sizeof(*addr->iso_bc));
+
+ if (!addr) {
+ ERROR_FAILED(err, "poll", ENOMEM);
+ return FALSE;
+ }
+
+ memset(addr, 0, sizeof(*addr) + sizeof(*addr->iso_bc));
+ addr->iso_family = AF_BLUETOOTH;
+
+ addr->iso_bc->bc_num_bis = bc_num_bis;
+ memcpy(addr->iso_bc->bc_bis, bc_bis,
+ addr->iso_bc->bc_num_bis);
+
+ if (bind(sock, (struct sockaddr *)addr,
+ sizeof(*addr) + sizeof(*addr->iso_bc)) < 0) {
+ ERROR_FAILED(err, "bind", errno);
+ return FALSE;
+ }
+ }
+
memset(&pfd, 0, sizeof(pfd));
pfd.fd = sock;
pfd.events = POLLOUT;
diff --git a/btio/btio.h b/btio/btio.h
index 3169bebf3..3e69092b1 100644
--- a/btio/btio.h
+++ b/btio/btio.h
@@ -77,7 +77,7 @@ gboolean bt_io_accept(GIOChannel *io, BtIOConnect connect, gpointer user_data,
gboolean bt_io_bcast_accept(GIOChannel *io, BtIOConnect connect,
gpointer user_data, GDestroyNotify destroy,
- GError **err);
+ GError **err, BtIOOption opt1, ...);
gboolean bt_io_set(GIOChannel *io, GError **err, BtIOOption opt1, ...);
diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index fa5cf1f54..b74498c4c 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -981,7 +981,7 @@ static void iso_pa_sync_confirm_cb(GIOChannel *io, void *user_data)
GError *err = NULL;
if (!bt_io_bcast_accept(io, iso_bcast_confirm_cb,
- user_data, NULL, &err)) {
+ user_data, NULL, &err, BT_IO_OPT_INVALID)) {
error("bt_io_bcast_accept: %s", err->message);
g_error_free(err);
g_io_channel_shutdown(io, TRUE, NULL);
diff --git a/src/shared/bass.c b/src/shared/bass.c
index 78103d463..326ce6dae 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -774,8 +774,9 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
if (bass_trigger_big_sync(bcast_src)) {
if (!bt_io_bcast_accept(bcast_src->pa_sync_io,
- connect_cb, bcast_src, NULL, &gerr)) {
- DBG(bcast_src->bass, "bt_io_accept: %s",
+ connect_cb, bcast_src, NULL, &gerr,
+ BT_IO_OPT_INVALID)) {
+ DBG(bcast_src->bass, "bt_io_bcast_accept: %s",
gerr->message);
g_error_free(gerr);
}
@@ -1178,8 +1179,8 @@ static void bass_handle_set_bcast_code_op(struct bt_bass *bass,
}
if (!bt_io_bcast_accept(bcast_src->pa_sync_io, connect_cb,
- bcast_src, NULL, &gerr)) {
- DBG(bcast_src->bass, "bt_io_accept: %s", gerr->message);
+ bcast_src, NULL, &gerr, BT_IO_OPT_INVALID)) {
+ DBG(bcast_src->bass, "bt_io_bcast_accept: %s", gerr->message);
g_error_free(gerr);
}
}
--
2.39.2
This adds an initial implementation for the BASS Modify Source
procedure
---
src/shared/bass.c | 289 +++++++++++++++++++++++++++++++++++++++++-----
src/shared/bass.h | 2 +
2 files changed, 263 insertions(+), 28 deletions(-)
diff --git a/src/shared/bass.c b/src/shared/bass.c
index 326ce6dae..5765ef6ed 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -614,6 +614,9 @@ static void connect_cb(GIOChannel *io, GError *gerr,
g_io_channel_ref(io);
queue_push_tail(bcast_src->bises, io);
+ if (!bcast_src->bises)
+ bcast_src->bises = queue_new();
+
for (i = 0; i < bcast_src->num_subgroups; i++) {
struct bt_bass_subgroup_data *data =
&bcast_src->subgroup_data[i];
@@ -650,29 +653,6 @@ static void connect_cb(GIOChannel *io, GError *gerr,
queue_destroy(bcast_src->bises, bass_bis_unref);
bcast_src->bises = NULL;
- /* Close listen io */
- g_io_channel_shutdown(bcast_src->listen_io, TRUE, NULL);
- g_io_channel_unref(bcast_src->listen_io);
- bcast_src->listen_io = NULL;
-
- if (bcast_src->listen_io_id > 0) {
- g_source_remove(bcast_src->listen_io_id);
- bcast_src->listen_io_id = 0;
- }
-
- /* Close pa sync io */
- if (bcast_src->pa_sync_io) {
- g_io_channel_shutdown(bcast_src->pa_sync_io,
- TRUE, NULL);
- g_io_channel_unref(bcast_src->pa_sync_io);
- bcast_src->pa_sync_io = NULL;
- }
-
- if (bcast_src->pa_sync_io_id > 0) {
- g_source_remove(bcast_src->pa_sync_io_id);
- bcast_src->pa_sync_io_id = 0;
- }
-
for (i = 0; i < bcast_src->num_subgroups; i++)
bcast_src->subgroup_data[i].bis_sync =
BT_BASS_BIG_SYNC_FAILED_BITMASK;
@@ -741,6 +721,17 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
/* Mark PA sync as failed and notify client */
bcast_src->sync_state = BT_BASS_FAILED_TO_SYNCHRONIZE_TO_PA;
+
+ /* Close listen io */
+ g_io_channel_shutdown(bcast_src->listen_io, TRUE, NULL);
+ g_io_channel_unref(bcast_src->listen_io);
+ bcast_src->listen_io = NULL;
+
+ if (bcast_src->listen_io_id > 0) {
+ g_source_remove(bcast_src->listen_io_id);
+ bcast_src->listen_io_id = 0;
+ }
+
goto notify;
}
@@ -1037,7 +1028,7 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
data->meta_len), data->meta_len);
}
- if (pa_sync != PA_SYNC_NO_SYNC) {
+ if (*pa_sync != PA_SYNC_NO_SYNC) {
/* Convert to three-value type */
if (bcast_src->addr_type)
addr_type = BDADDR_LE_RANDOM;
@@ -1072,9 +1063,6 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
G_IO_HUP | G_IO_NVAL,
(GIOFunc)listen_io_disconnect_cb,
bcast_src);
-
- if (num_bis > 0 && !bcast_src->bises)
- bcast_src->bises = queue_new();
} else {
for (int i = 0; i < bcast_src->num_subgroups; i++)
bcast_src->subgroup_data[i].bis_sync =
@@ -1179,12 +1167,255 @@ static void bass_handle_set_bcast_code_op(struct bt_bass *bass,
}
if (!bt_io_bcast_accept(bcast_src->pa_sync_io, connect_cb,
- bcast_src, NULL, &gerr, BT_IO_OPT_INVALID)) {
+ bcast_src, NULL, &gerr, BT_IO_OPT_INVALID)) {
DBG(bcast_src->bass, "bt_io_bcast_accept: %s", gerr->message);
g_error_free(gerr);
}
}
+static bool bass_validate_mod_src_params(struct bt_bass_mod_src_params *params,
+ struct iovec *iov)
+{
+ struct iovec subgroup_data = {
+ .iov_base = iov->iov_base,
+ .iov_len = iov->iov_len,
+ };
+
+ if (params->pa_sync > PA_SYNC_NO_PAST)
+ return false;
+
+ if (!bass_validate_bis_sync(params->num_subgroups,
+ &subgroup_data))
+ return false;
+
+ return true;
+}
+
+static void bass_handle_mod_src_op(struct bt_bass *bass,
+ struct gatt_db_attribute *attrib,
+ uint8_t opcode,
+ unsigned int id,
+ struct iovec *iov,
+ struct bt_att *att)
+{
+ struct bt_bass_mod_src_params *params;
+ struct bt_bcast_src *bcast_src;
+ uint8_t num_bis = 0;
+ uint8_t bis[ISO_MAX_NUM_BIS];
+ uint8_t addr_type;
+ GIOChannel *io;
+ GError *err = NULL;
+ struct bt_iso_qos iso_qos = default_qos;
+ struct bt_bass_subgroup_data *subgroup_data = NULL;
+ uint8_t *notify_data;
+ size_t notify_data_len;
+ bool changed = false;
+
+ /* Get Modify Source command parameters */
+ params = util_iov_pull_mem(iov, sizeof(*params));
+
+ bcast_src = queue_find(bass->ldb->bcast_srcs,
+ bass_src_id_match,
+ ¶ms->id);
+
+ if (!bcast_src) {
+ /* No source matches the written source id */
+ gatt_db_attribute_write_result(attrib, id,
+ BT_BASS_ERROR_INVALID_SOURCE_ID);
+
+ return;
+ }
+
+ gatt_db_attribute_write_result(attrib, id, 0x00);
+
+ /* Ignore operation if parameters are invalid */
+ if (!bass_validate_mod_src_params(params, iov))
+ return;
+
+ /* Only subgroup metadata can change */
+ if (params->num_subgroups != bcast_src->num_subgroups)
+ return;
+
+ /* Extract subgroup information */
+ subgroup_data = malloc(params->num_subgroups * sizeof(*subgroup_data));
+ if (!subgroup_data)
+ return;
+
+ memset(subgroup_data, 0, params->num_subgroups *
+ sizeof(*subgroup_data));
+
+ for (int i = 0; i < params->num_subgroups; i++) {
+ struct bt_bass_subgroup_data *data = &subgroup_data[i];
+ struct bt_bass_subgroup_data *old_data =
+ &bcast_src->subgroup_data[i];
+
+ util_iov_pull_le32(iov, &data->pending_bis_sync);
+
+ if (old_data->pending_bis_sync)
+ goto err;
+
+ if (old_data->bis_sync && data->pending_bis_sync &&
+ data->pending_bis_sync != old_data->bis_sync)
+ /* The Server cannot resync to different BIS indxes */
+ goto err;
+
+ if (data->pending_bis_sync == BIS_SYNC_NO_PREF)
+ data->pending_bis_sync = DEFAULT_BIS_SYNC_BITMASK;
+
+ if (data->pending_bis_sync != 0) {
+ /* Client instructs Server to sync to some BISes */
+ for (int bis_idx = 0; bis_idx < 31; bis_idx++) {
+ if (data->pending_bis_sync & (1 << bis_idx)) {
+ bis[num_bis] = bis_idx + 1;
+ num_bis++;
+ }
+ }
+ }
+
+ data->meta_len = *(uint8_t *)util_iov_pull_mem(iov,
+ sizeof(data->meta_len));
+ if (!data->meta_len)
+ continue;
+
+ data->meta = malloc(data->meta_len);
+ if (!data->meta)
+ goto err;
+
+ memcpy(data->meta, (uint8_t *)util_iov_pull_mem(iov,
+ data->meta_len), data->meta_len);
+
+ if (old_data->meta_len != data->meta_len ||
+ memcmp(old_data->meta, data->meta, old_data->meta_len))
+ changed = true;
+ }
+
+ if (bcast_src->sync_state == BT_BASS_NOT_SYNCHRONIZED_TO_PA &&
+ bcast_src->listen_io)
+ return;
+
+ /* If subgroup information has been extracted successfully,
+ * update broadcast source
+ */
+ if (bcast_src->subgroup_data) {
+ for (int i = 0; i < bcast_src->num_subgroups; i++)
+ free(bcast_src->subgroup_data[i].meta);
+
+ free(bcast_src->subgroup_data);
+ }
+
+ bcast_src->subgroup_data = subgroup_data;
+
+ /* Client instructs Server to sync to PA */
+ if (bcast_src->sync_state == BT_BASS_NOT_SYNCHRONIZED_TO_PA) {
+ if (params->pa_sync == PA_SYNC_NO_PAST) {
+ /* Convert to three-value type */
+ if (bcast_src->addr_type)
+ addr_type = BDADDR_LE_RANDOM;
+ else
+ addr_type = BDADDR_LE_PUBLIC;
+
+ /* Try to synchronize to the source */
+ io = bt_io_listen(NULL, confirm_cb, bcast_src,
+ NULL, &err,
+ BT_IO_OPT_SOURCE_BDADDR,
+ &bass->ldb->adapter_bdaddr,
+ BT_IO_OPT_DEST_BDADDR,
+ &bcast_src->addr,
+ BT_IO_OPT_DEST_TYPE,
+ addr_type,
+ BT_IO_OPT_MODE, BT_IO_MODE_ISO,
+ BT_IO_OPT_QOS, &iso_qos,
+ BT_IO_OPT_ISO_BC_SID,
+ bcast_src->sid,
+ BT_IO_OPT_ISO_BC_NUM_BIS,
+ num_bis,
+ BT_IO_OPT_ISO_BC_BIS, bis,
+ BT_IO_OPT_INVALID);
+
+ if (!io) {
+ DBG(bass, "%s", err->message);
+ g_error_free(err);
+ return;
+ }
+
+ bcast_src->listen_io = io;
+ g_io_channel_ref(bcast_src->listen_io);
+
+ if (bcast_src->listen_io_id > 0) {
+ g_source_remove(bcast_src->listen_io_id);
+ bcast_src->listen_io_id = 0;
+ }
+
+ bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
+ G_IO_HUP | G_IO_NVAL,
+ (GIOFunc)listen_io_disconnect_cb,
+ bcast_src);
+ }
+
+ if (changed)
+ goto notify;
+
+ return;
+ }
+
+ /* Client instructs Server to stop PA sync */
+ if (params->pa_sync == PA_SYNC_NO_SYNC) {
+ /* TODO */
+ return;
+ }
+
+ /* Server is already synchronized to PA, and Client now
+ * instructs the Server to also sync to some BISes
+ */
+ if (num_bis && !queue_length(bcast_src->bises)) {
+ /* Since the Server has never been synced to any BIS before,
+ * the PA sync socket must be bound to the num_bis to sync with
+ */
+ if (!bt_io_bcast_accept(bcast_src->pa_sync_io, connect_cb,
+ bcast_src, NULL, &err,
+ BT_IO_OPT_ISO_BC_NUM_BIS, num_bis,
+ BT_IO_OPT_ISO_BC_BIS, bis,
+ BT_IO_OPT_INVALID)) {
+ DBG(bcast_src->bass, "bt_io_bcast_accept: %s",
+ err->message);
+ g_error_free(err);
+ }
+
+ return;
+ }
+
+ /* Client instructs Server to keep PA sync but stop BIG sync */
+ if (!num_bis && queue_length(bcast_src->bises)) {
+ /* TODO */
+ return;
+ }
+
+ /* If no change has occurred, just return */
+ if (!changed)
+ return;
+
+notify:
+ notify_data = bass_build_notif_from_bcast_src(bcast_src,
+ ¬ify_data_len);
+
+ gatt_db_attribute_notify(bcast_src->attr,
+ (void *)notify_data,
+ notify_data_len,
+ bt_bass_get_att(bcast_src->bass));
+
+ free(notify_data);
+
+ return;
+
+err:
+ if (subgroup_data) {
+ for (int i = 0; i < params->num_subgroups; i++)
+ free(subgroup_data[i].meta);
+
+ free(subgroup_data);
+ }
+}
+
#define BASS_OP(_str, _op, _size, _func) \
{ \
.str = _str, \
@@ -1214,6 +1445,8 @@ struct bass_op_handler {
0, bass_handle_add_src_op),
BASS_OP("Set Broadcast Code", BT_BASS_SET_BCAST_CODE,
0, bass_handle_set_bcast_code_op),
+ BASS_OP("Modify Source", BT_BASS_MOD_SRC,
+ 0, bass_handle_mod_src_op),
{}
};
diff --git a/src/shared/bass.h b/src/shared/bass.h
index bd3fe900b..ee958bf7a 100644
--- a/src/shared/bass.h
+++ b/src/shared/bass.h
@@ -32,6 +32,8 @@ struct bt_bass;
#define BT_BASS_BIG_ENC_STATE_DEC 0x02
#define BT_BASS_BIG_ENC_STATE_BAD_CODE 0x03
+#define DEFAULT_BIS_SYNC_BITMASK 0x00000001
+
/* BASS subgroup field of the Broadcast
* Receive State characteristic
*/
--
2.39.2
Hi Iulia,
On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <[email protected]> wrote:
>
> This handles G_IO_ERR and G_IO_HUP conditions in server_cb
>
> ---
> btio/btio.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index d45b8240d..c63a6d1df 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io, GIOCondition cond,
> GIOChannel *cli_io;
>
> /* If the user closed the server */
> - if ((cond & G_IO_NVAL) || check_nval(io))
> + if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
> + check_nval(io))
I believe this was done on purpose to only check for NVAL, the other
conditions shall probably be notified back via callback so it can
process the error reported.
> return FALSE;
>
> srv_sock = g_io_channel_unix_get_fd(io);
> --
> 2.39.2
>
--
Luiz Augusto von Dentz
Hi Iulia,
On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <[email protected]> wrote:
>
> This adds watches to handle closed sockets
>
> ---
> src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/bass.h | 2 ++
> 2 files changed, 61 insertions(+)
>
> diff --git a/src/shared/bass.c b/src/shared/bass.c
> index 0ee3187d1..78103d463 100644
> --- a/src/shared/bass.c
> +++ b/src/shared/bass.c
> @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
> g_io_channel_unref(bcast_src->listen_io);
> bcast_src->listen_io = NULL;
>
> + if (bcast_src->listen_io_id > 0) {
> + g_source_remove(bcast_src->listen_io_id);
> + bcast_src->listen_io_id = 0;
> + }
> +
> /* Close pa sync io */
> if (bcast_src->pa_sync_io) {
> g_io_channel_shutdown(bcast_src->pa_sync_io,
> @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
> bcast_src->pa_sync_io = NULL;
> }
>
> + if (bcast_src->pa_sync_io_id > 0) {
> + g_source_remove(bcast_src->pa_sync_io_id);
> + bcast_src->pa_sync_io_id = 0;
> + }
> +
> for (i = 0; i < bcast_src->num_subgroups; i++)
> bcast_src->subgroup_data[i].bis_sync =
> BT_BASS_BIG_SYNC_FAILED_BITMASK;
> @@ -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src *bcast_src)
> return false;
> }
>
> +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
> + gpointer data)
> +{
> + struct bt_bcast_src *bcast_src = data;
> +
> + DBG(bcast_src->bass, "PA sync io has been disconnected");
> +
> + bcast_src->pa_sync_io_id = 0;
> + bcast_src->pa_sync_io = NULL;
> +
> + return FALSE;
> +}
>
> static void confirm_cb(GIOChannel *io, gpointer user_data)
> {
> @@ -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
> bcast_src->pa_sync_io = io;
> g_io_channel_ref(bcast_src->pa_sync_io);
>
> + if (bcast_src->pa_sync_io_id > 0) {
> + g_source_remove(bcast_src->pa_sync_io_id);
> + bcast_src->pa_sync_io_id = 0;
> + }
> +
> + bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR | G_IO_HUP |
> + G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
> + bcast_src);
> +
> len = sizeof(qos);
> memset(&qos, 0, len);
>
> @@ -844,6 +875,19 @@ static bool bass_validate_add_src_params(uint8_t *value, size_t len)
> return true;
> }
>
> +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
> + gpointer data)
> +{
> + struct bt_bcast_src *bcast_src = data;
> +
> + DBG(bcast_src->bass, "Listen io has been disconnected");
> +
> + bcast_src->listen_io_id = 0;
> + bcast_src->listen_io = NULL;
> +
> + return FALSE;
> +}
> +
> static void bass_handle_add_src_op(struct bt_bass *bass,
> struct gatt_db_attribute *attrib,
> uint8_t opcode,
> @@ -1023,6 +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
> bcast_src->listen_io = io;
> g_io_channel_ref(bcast_src->listen_io);
>
> + bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
> + G_IO_HUP | G_IO_NVAL,
> + (GIOFunc)listen_io_disconnect_cb,
> + bcast_src);
IO handling shall probably be kept outside of shared so we could
emulate it with use of socketpairs and unit test it, see how it was
done for bap.
> if (num_bis > 0 && !bcast_src->bises)
> bcast_src->bises = queue_new();
> } else {
> @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
> g_io_channel_unref(bcast_src->listen_io);
> }
>
> + if (bcast_src->listen_io_id > 0) {
> + g_source_remove(bcast_src->listen_io_id);
> + bcast_src->listen_io_id = 0;
> + }
> +
> if (bcast_src->pa_sync_io) {
> g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
> g_io_channel_unref(bcast_src->pa_sync_io);
> }
>
> + if (bcast_src->pa_sync_io_id > 0) {
> + g_source_remove(bcast_src->pa_sync_io_id);
> + bcast_src->pa_sync_io_id = 0;
> + }
> +
> queue_destroy(bcast_src->bises, bass_bis_unref);
>
> free(bcast_src);
> diff --git a/src/shared/bass.h b/src/shared/bass.h
> index c4b5b76ba..bd3fe900b 100644
> --- a/src/shared/bass.h
> +++ b/src/shared/bass.h
> @@ -57,7 +57,9 @@ struct bt_bcast_src {
> uint8_t num_subgroups;
> struct bt_bass_subgroup_data *subgroup_data;
> GIOChannel *listen_io;
> + guint listen_io_id;
> GIOChannel *pa_sync_io;
> + guint pa_sync_io_id;
> struct queue *bises;
> };
>
> --
> 2.39.2
>
--
Luiz Augusto von Dentz
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=793607
---Test result---
Test Summary:
CheckPatch PASS 2.06 seconds
GitLint PASS 1.10 seconds
BuildEll PASS 27.73 seconds
BluezMake PASS 798.70 seconds
MakeCheck PASS 12.22 seconds
MakeDistcheck PASS 175.13 seconds
CheckValgrind PASS 267.39 seconds
CheckSmatch PASS 360.77 seconds
bluezmakeextell PASS 115.59 seconds
IncrementalBuild PASS 2814.91 seconds
ScanBuild WARNING 1102.61 seconds
Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
btio/btio.c:1852:4: warning: Potential leak of memory pointed to by 'addr'
ERROR_FAILED(err, "bind", errno);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
btio/btio.c:41:3: note: expanded from macro 'ERROR_FAILED'
g_set_error(gerr, BT_IO_ERROR, err, \
^~~~~~~~~~~
1 warning generated.
src/shared/bass.c:1294:3: warning: Potential leak of memory pointed to by 'subgroup_data'
return;
^~~~~~
1 warning generated.
---
Regards,
Linux Bluetooth
Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Monday, October 16, 2023 8:01 PM
> To: Iulia Tanasescu <[email protected]>
> Cc: [email protected]; Claudia Cristina Draghicescu
> <[email protected]>; Mihai-Octavian Urzica <mihai-
> [email protected]>; Silviu Florian Barbulescu
> <[email protected]>; Vlad Pruteanu <[email protected]>;
> Andrei Istodorescu <[email protected]>
> Subject: Re: [PATCH BlueZ 1/4] btio: Handle closed channel in server_cb
>
> Hi Iulia,
>
> On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <[email protected]>
> wrote:
> >
> > This handles G_IO_ERR and G_IO_HUP conditions in server_cb
> >
> > ---
> > btio/btio.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/btio/btio.c b/btio/btio.c index d45b8240d..c63a6d1df
> > 100644
> > --- a/btio/btio.c
> > +++ b/btio/btio.c
> > @@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io,
> GIOCondition cond,
> > GIOChannel *cli_io;
> >
> > /* If the user closed the server */
> > - if ((cond & G_IO_NVAL) || check_nval(io))
> > + if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
> > + check_nval(io))
>
> I believe this was done on purpose to only check for NVAL, the other
> conditions shall probably be notified back via callback so it can process the
> error reported.
>
It seems that server_cb is supposed to accept an incoming connection on
server io and pass the child io through the confirm or connect callbacks.
But if the condition is G_IO_HUP for example, we shouldn't get to the point
of calling accept.
> > return FALSE;
> >
> > srv_sock = g_io_channel_unix_get_fd(io);
> > --
> > 2.39.2
> >
>
>
> --
> Luiz Augusto von Dentz
Regards,
Iulia
Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Monday, October 16, 2023 8:05 PM
> To: Iulia Tanasescu <[email protected]>
> Cc: [email protected]; Claudia Cristina Draghicescu
> <[email protected]>; Mihai-Octavian Urzica <mihai-
> [email protected]>; Silviu Florian Barbulescu
> <[email protected]>; Vlad Pruteanu <[email protected]>;
> Andrei Istodorescu <[email protected]>
> Subject: Re: [PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io
> channels
>
> Hi Iulia,
>
> On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <[email protected]>
> wrote:
> >
> > This adds watches to handle closed sockets
> >
> > ---
> > src/shared/bass.c | 59
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > src/shared/bass.h | 2 ++
> > 2 files changed, 61 insertions(+)
> >
> > diff --git a/src/shared/bass.c b/src/shared/bass.c index
> > 0ee3187d1..78103d463 100644
> > --- a/src/shared/bass.c
> > +++ b/src/shared/bass.c
> > @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError
> *gerr,
> > g_io_channel_unref(bcast_src->listen_io);
> > bcast_src->listen_io = NULL;
> >
> > + if (bcast_src->listen_io_id > 0) {
> > + g_source_remove(bcast_src->listen_io_id);
> > + bcast_src->listen_io_id = 0;
> > + }
> > +
> > /* Close pa sync io */
> > if (bcast_src->pa_sync_io) {
> > g_io_channel_shutdown(bcast_src->pa_sync_io,
> > @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError
> *gerr,
> > bcast_src->pa_sync_io = NULL;
> > }
> >
> > + if (bcast_src->pa_sync_io_id > 0) {
> > + g_source_remove(bcast_src->pa_sync_io_id);
> > + bcast_src->pa_sync_io_id = 0;
> > + }
> > +
> > for (i = 0; i < bcast_src->num_subgroups; i++)
> > bcast_src->subgroup_data[i].bis_sync =
> > BT_BASS_BIG_SYNC_FAILED_BITMASK; @@
> > -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src
> *bcast_src)
> > return false;
> > }
> >
> > +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io,
> GIOCondition cond,
> > + gpointer data) {
> > + struct bt_bcast_src *bcast_src = data;
> > +
> > + DBG(bcast_src->bass, "PA sync io has been disconnected");
> > +
> > + bcast_src->pa_sync_io_id = 0;
> > + bcast_src->pa_sync_io = NULL;
> > +
> > + return FALSE;
> > +}
> >
> > static void confirm_cb(GIOChannel *io, gpointer user_data) { @@
> > -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer
> user_data)
> > bcast_src->pa_sync_io = io;
> > g_io_channel_ref(bcast_src->pa_sync_io);
> >
> > + if (bcast_src->pa_sync_io_id > 0) {
> > + g_source_remove(bcast_src->pa_sync_io_id);
> > + bcast_src->pa_sync_io_id = 0;
> > + }
> > +
> > + bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR |
> G_IO_HUP |
> > + G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
> > + bcast_src);
> > +
> > len = sizeof(qos);
> > memset(&qos, 0, len);
> >
> > @@ -844,6 +875,19 @@ static bool
> bass_validate_add_src_params(uint8_t *value, size_t len)
> > return true;
> > }
> >
> > +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition
> cond,
> > + gpointer data) {
> > + struct bt_bcast_src *bcast_src = data;
> > +
> > + DBG(bcast_src->bass, "Listen io has been disconnected");
> > +
> > + bcast_src->listen_io_id = 0;
> > + bcast_src->listen_io = NULL;
> > +
> > + return FALSE;
> > +}
> > +
> > static void bass_handle_add_src_op(struct bt_bass *bass,
> > struct gatt_db_attribute *attrib,
> > uint8_t opcode, @@ -1023,6
> > +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
> > bcast_src->listen_io = io;
> > g_io_channel_ref(bcast_src->listen_io);
> >
> > + bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
> > + G_IO_HUP | G_IO_NVAL,
> > + (GIOFunc)listen_io_disconnect_cb,
> > + bcast_src);
>
> IO handling shall probably be kept outside of shared so we could emulate it
> with use of socketpairs and unit test it, see how it was done for bap.
>
I submitted a patch to move all IO handling from src/shared to
profiles/audio, similar to bap.
> > if (num_bis > 0 && !bcast_src->bises)
> > bcast_src->bises = queue_new();
> > } else {
> > @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
> > g_io_channel_unref(bcast_src->listen_io);
> > }
> >
> > + if (bcast_src->listen_io_id > 0) {
> > + g_source_remove(bcast_src->listen_io_id);
> > + bcast_src->listen_io_id = 0;
> > + }
> > +
> > if (bcast_src->pa_sync_io) {
> > g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
> > g_io_channel_unref(bcast_src->pa_sync_io);
> > }
> >
> > + if (bcast_src->pa_sync_io_id > 0) {
> > + g_source_remove(bcast_src->pa_sync_io_id);
> > + bcast_src->pa_sync_io_id = 0;
> > + }
> > +
> > queue_destroy(bcast_src->bises, bass_bis_unref);
> >
> > free(bcast_src);
> > diff --git a/src/shared/bass.h b/src/shared/bass.h index
> > c4b5b76ba..bd3fe900b 100644
> > --- a/src/shared/bass.h
> > +++ b/src/shared/bass.h
> > @@ -57,7 +57,9 @@ struct bt_bcast_src {
> > uint8_t num_subgroups;
> > struct bt_bass_subgroup_data *subgroup_data;
> > GIOChannel *listen_io;
> > + guint listen_io_id;
> > GIOChannel *pa_sync_io;
> > + guint pa_sync_io_id;
> > struct queue *bises;
> > };
> >
> > --
> > 2.39.2
> >
>
>
> --
> Luiz Augusto von Dentz
Regards,
Iulia