2023-03-01 01:38:48

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 1/5] device: Don't attempt to connect LE if ATT is already connected

From: Luiz Augusto von Dentz <[email protected]>

This checks if an att instance already exists before attempting to
connect it once again.
---
src/device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/device.c b/src/device.c
index d270421cc7c9..cb16d37c1ae1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -5422,7 +5422,7 @@ int device_connect_le(struct btd_device *dev)
char addr[18];

/* There is one connection attempt going on */
- if (dev->att_io)
+ if (dev->att_io || dev->att)
return -EALREADY;

ba2str(&dev->bdaddr, addr);
--
2.39.2



2023-03-01 01:38:50

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 2/5] client: Allow transport.send command to work with multiple transports

From: Luiz Augusto von Dentz <[email protected]>

This enables transport.send to work with multiple transports instead of
sending one by one which can create synchronization problems.
---
client/player.c | 71 +++++++++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/client/player.c b/client/player.c
index 65cac3b50376..767304b567b6 100644
--- a/client/player.c
+++ b/client/player.c
@@ -3566,48 +3566,54 @@ static void cmd_send_transport(int argc, char *argv[])
{
GDBusProxy *proxy;
struct transport *transport;
- int fd, err;
+ int fd = -1, err;
struct bt_iso_qos qos;
socklen_t len;
+ int i;

- proxy = g_dbus_proxy_lookup(transports, NULL, argv[1],
+ for (i = 1; i < argc; i++) {
+ proxy = g_dbus_proxy_lookup(transports, NULL, argv[i],
BLUEZ_MEDIA_TRANSPORT_INTERFACE);
- if (!proxy) {
- bt_shell_printf("Transport %s not found\n", argv[1]);
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
- }
+ if (!proxy) {
+ bt_shell_printf("Transport %s not found\n", argv[i]);
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }

- transport = find_transport(proxy);
- if (!transport) {
- bt_shell_printf("Transport %s not acquired\n", argv[1]);
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
- }
+ transport = find_transport(proxy);
+ if (!transport) {
+ bt_shell_printf("Transport %s not acquired\n", argv[i]);
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }

- if (transport->sk < 0) {
- bt_shell_printf("No Transport Socked found\n");
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
- }
+ if (transport->sk < 0) {
+ bt_shell_printf("No Transport Socked found\n");
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }

- fd = open_file(argv[2], O_RDONLY);
- if (fd < 0)
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ if (i + 1 < argc) {
+ fd = open_file(argv[++i], O_RDONLY);
+ if (fd < 0)
+ return bt_shell_noninteractive_quit(
+ EXIT_FAILURE);
+ }

- bt_shell_printf("Sending ...\n");
+ bt_shell_printf("Sending ...\n");

- /* Read QoS if available */
- memset(&qos, 0, sizeof(qos));
- len = sizeof(qos);
- if (getsockopt(transport->sk, SOL_BLUETOOTH, BT_ISO_QOS, &qos,
+ /* Read QoS if available */
+ memset(&qos, 0, sizeof(qos));
+ len = sizeof(qos);
+ if (getsockopt(transport->sk, SOL_BLUETOOTH, BT_ISO_QOS, &qos,
&len) < 0)
- err = transport_send(transport, fd, NULL);
- else
- err = transport_send(transport, fd, &qos);
+ err = transport_send(transport, fd, NULL);
+ else
+ err = transport_send(transport, fd, &qos);

- if (err < 0) {
- bt_shell_printf("Unable to send: %s (%d)", strerror(-err),
- -err);
- close(fd);
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ if (err < 0) {
+ bt_shell_printf("Unable to send: %s (%d)",
+ strerror(-err), -err);
+ close(fd);
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }
}

return bt_shell_noninteractive_quit(EXIT_SUCCESS);
@@ -3710,7 +3716,8 @@ static const struct bt_shell_menu transport_menu = {
{ "release", "<transport> [transport1...]", cmd_release_transport,
"Release Transport",
transport_generator },
- { "send", "<transport> <filename>", cmd_send_transport,
+ { "send", "<transport> <filename> [transport1...]",
+ cmd_send_transport,
"Send contents of a file",
transport_generator },
{ "receive", "<transport> [filename]", cmd_receive_transport,
--
2.39.2


2023-03-01 01:38:51

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 3/5] shared/bap: Cleanup requests on detach

From: Luiz Augusto von Dentz <[email protected]>

If session is being detached any ongoing/queue request shall be
cancelled as well otherwise when the session is attach again they would
be invalid.
---
src/shared/bap.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index f16ba1832aaa..c63612f0da47 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -3820,6 +3820,13 @@ static void stream_foreach_detach(void *data, void *user_data)
stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
}

+static void bap_req_detach(void *data)
+{
+ struct bt_bap_req *req = data;
+
+ bap_req_complete(req, NULL);
+}
+
void bt_bap_detach(struct bt_bap *bap)
{
DBG(bap, "%p", bap);
@@ -3827,6 +3834,15 @@ void bt_bap_detach(struct bt_bap *bap)
if (!queue_remove(sessions, bap))
return;

+ /* Cancel ongoing request */
+ if (bap->req) {
+ bap_req_detach(bap->req);
+ bap->req = NULL;
+ }
+
+ /* Cancel queued requests */
+ queue_remove_all(bap->reqs, NULL, NULL, bap_req_detach);
+
bt_gatt_client_unref(bap->client);
bap->client = NULL;

--
2.39.2


2023-03-01 01:38:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 4/5] share/gatt-client: Introduce idle callback

From: Luiz Augusto von Dentz <[email protected]>

This introduces the concept of idle callback which can be used to get
notified when there is no more pending requests by the client.
---
src/shared/gatt-client.c | 78 ++++++++++++++++++++++++++++++++++++++--
src/shared/gatt-client.h | 8 +++++
2 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index baff68f28c65..f885076913dc 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -47,6 +47,12 @@ struct ready_cb {
void *data;
};

+struct idle_cb {
+ bt_gatt_client_idle_callback_t callback;
+ bt_gatt_client_destroy_func_t destroy;
+ void *data;
+};
+
struct bt_gatt_client {
struct bt_att *att;
int ref_count;
@@ -56,6 +62,7 @@ struct bt_gatt_client {
struct queue *clones;

struct queue *ready_cbs;
+ struct queue *idle_cbs;

bt_gatt_client_service_changed_callback_t svc_chngd_callback;
bt_gatt_client_destroy_func_t svc_chngd_destroy;
@@ -147,9 +154,38 @@ static struct request *request_create(struct bt_gatt_client *client)
return request_ref(req);
}

+static void idle_destroy(void *data)
+{
+ struct idle_cb *idle = data;
+
+ if (idle->destroy)
+ idle->destroy(idle->data);
+
+ free(idle);
+}
+
+static bool idle_notify(const void *data, const void *user_data)
+{
+ const struct idle_cb *idle = data;
+
+ idle->callback(idle->data);
+
+ return true;
+}
+
+static void notify_client_idle(struct bt_gatt_client *client)
+{
+ bt_gatt_client_ref(client);
+
+ queue_remove_all(client->idle_cbs, idle_notify, NULL, idle_destroy);
+
+ bt_gatt_client_unref(client);
+}
+
static void request_unref(void *data)
{
struct request *req = data;
+ struct bt_gatt_client *client = req->client;

if (__sync_sub_and_fetch(&req->ref_count, 1))
return;
@@ -157,8 +193,11 @@ static void request_unref(void *data)
if (req->destroy)
req->destroy(req->data);

- if (!req->removed)
- queue_remove(req->client->pending_requests, req);
+ if (!req->removed) {
+ queue_remove(client->pending_requests, req);
+ if (queue_isempty(client->pending_requests))
+ notify_client_idle(client);
+ }

free(req);
}
@@ -2234,6 +2273,7 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
queue_destroy(client->notify_list, notify_data_cleanup);

queue_destroy(client->ready_cbs, ready_destroy);
+ queue_destroy(client->idle_cbs, idle_destroy);

if (client->debug_destroy)
client->debug_destroy(client->debug_data);
@@ -2292,6 +2332,7 @@ static struct bt_gatt_client *gatt_client_new(struct gatt_db *db,

client->clones = queue_new();
client->ready_cbs = queue_new();
+ client->idle_cbs = queue_new();
client->long_write_queue = queue_new();
client->svc_chngd_queue = queue_new();
client->notify_list = queue_new();
@@ -3727,3 +3768,36 @@ int bt_gatt_client_get_security(struct bt_gatt_client *client)

return bt_att_get_security(client->att, NULL);
}
+
+unsigned int bt_gatt_client_idle_register(struct bt_gatt_client *client,
+ bt_gatt_client_idle_callback_t callback,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy)
+{
+ struct idle_cb *idle;
+
+ if (!client)
+ return 0;
+
+ idle = new0(struct idle_cb, 1);
+ idle->callback = callback;
+ idle->destroy = destroy;
+ idle->data = user_data;
+
+ queue_push_tail(client->idle_cbs, idle);
+
+ return PTR_TO_UINT(idle);
+}
+
+bool bt_gatt_client_idle_unregister(struct bt_gatt_client *client,
+ unsigned int id)
+{
+ struct idle_cb *idle = UINT_TO_PTR(id);
+
+ if (queue_remove(client->idle_cbs, idle)) {
+ idle_destroy(idle);
+ return true;
+ }
+
+ return false;
+}
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index dc51023942ca..bccd04a62003 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -26,6 +26,7 @@ struct bt_gatt_client *bt_gatt_client_ref(struct bt_gatt_client *client);
void bt_gatt_client_unref(struct bt_gatt_client *client);

typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
+typedef void (*bt_gatt_client_idle_callback_t)(void *user_data);
typedef void (*bt_gatt_client_callback_t)(bool success, uint8_t att_ecode,
void *user_data);
typedef void (*bt_gatt_client_debug_func_t)(const char *str, void *user_data);
@@ -126,3 +127,10 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,

bool bt_gatt_client_set_security(struct bt_gatt_client *client, int level);
int bt_gatt_client_get_security(struct bt_gatt_client *client);
+
+unsigned int bt_gatt_client_idle_register(struct bt_gatt_client *client,
+ bt_gatt_client_idle_callback_t callback,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy);
+bool bt_gatt_client_idle_unregister(struct bt_gatt_client *client,
+ unsigned int id);
--
2.39.2


2023-03-01 01:38:54

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 5/5] media: Fix not checking BREDR support for A2DP

From: Luiz Augusto von Dentz <[email protected]>

A2DP shall depend on MGMT_SETTING_BREDR setting so the likes of
bluetoothctl -e don't attempt to register A2DP with controller that
are on LE only mode.
---
profiles/audio/media.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 326e50a0925b..540e91bc6706 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1269,8 +1269,11 @@ static bool endpoint_properties_get(const char *uuid,
return true;
}

-static bool endpoint_supported(struct btd_adapter *adapter)
+static bool a2dp_endpoint_supported(struct btd_adapter *adapter)
{
+ if (!btd_adapter_has_settings(adapter, MGMT_SETTING_BREDR))
+ return false;
+
return true;
}

@@ -1291,8 +1294,10 @@ static struct media_endpoint_init {
bool (*func)(struct media_endpoint *endpoint, int *err);
bool (*supported)(struct btd_adapter *adapter);
} init_table[] = {
- { A2DP_SOURCE_UUID, endpoint_init_a2dp_source, endpoint_supported },
- { A2DP_SINK_UUID, endpoint_init_a2dp_sink, endpoint_supported },
+ { A2DP_SOURCE_UUID, endpoint_init_a2dp_source,
+ a2dp_endpoint_supported },
+ { A2DP_SINK_UUID, endpoint_init_a2dp_sink,
+ a2dp_endpoint_supported },
{ PAC_SINK_UUID, endpoint_init_pac_sink,
experimental_endpoint_supported },
{ PAC_SOURCE_UUID, endpoint_init_pac_source,
--
2.39.2


2023-03-01 04:58:30

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ,1/5] device: Don't attempt to connect LE if ATT is already connected

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=725670

---Test result---

Test Summary:
CheckPatch PASS 1.85 seconds
GitLint FAIL 1.79 seconds
BuildEll PASS 30.84 seconds
BluezMake PASS 1036.66 seconds
MakeCheck PASS 11.64 seconds
MakeDistcheck PASS 170.69 seconds
CheckValgrind PASS 280.89 seconds
CheckSmatch PASS 382.89 seconds
bluezmakeextell PASS 112.89 seconds
IncrementalBuild PASS 4390.29 seconds
ScanBuild WARNING 1224.48 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,2/5] client: Allow transport.send command to work with multiple transports

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (81>80): "[BlueZ,2/5] client: Allow transport.send command to work with multiple transports"
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
src/shared/gatt-client.c:440:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:685:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:982:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1088:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1280:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1345:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1616:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1621:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2127:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2135:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3223:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3245:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.



---
Regards,
Linux Bluetooth

2023-03-02 20:31:47

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/5] device: Don't attempt to connect LE if ATT is already connected

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Tue, 28 Feb 2023 17:38:38 -0800 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This checks if an att instance already exists before attempting to
> connect it once again.
> ---
> src/device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Here is the summary with links:
- [BlueZ,1/5] device: Don't attempt to connect LE if ATT is already connected
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1923f5dd9068
- [BlueZ,2/5] client: Allow transport.send command to work with multiple transports
(no matching commit)
- [BlueZ,3/5] shared/bap: Cleanup requests on detach
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=36c234625bda
- [BlueZ,4/5] share/gatt-client: Introduce idle callback
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d6f790f82de1
- [BlueZ,5/5] media: Fix not checking BREDR support for A2DP
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=15458c5e1071

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html