[PATCH BlueZ v6 2/2] profiles: centralize reflector setup

Naga Bhavani Akella <[email protected]> Mon, 27 Jul 2026 12:29:54 +0530
Newsgroups org.kernel.vger.linux-bluetooth
Message-ID <[email protected]>
Add a helper for reflector initialization and track D-Bus registration
state in rap_data. Simplify connection handling, move interface cleanup
to object destruction, and streamline removal logic.
---
 profiles/ranging/rap.c | 209 ++++++++++++++++++++++++++++-------------
 1 file changed, 144 insertions(+), 65 deletions(-)

diff --git a/profiles/ranging/rap.c b/profiles/ranging/rap.c
index 082c35558..91f30ac30 100644
--- a/profiles/ranging/rap.c
+++ b/profiles/ranging/rap.c
@@ -65,10 +65,12 @@ struct rap_data {
 	void *hci_sm;  /* Per-device HCI state machine */
 	uint16_t conn_handle;  /* Last known connection handle */
 	struct cs_session active_session;  /* active==false when idle */
+	bool dbus_registered;
 };
 
 static struct queue *sessions;
 static struct queue *adapter_list;  /* List of rap_adapter_data */
+static int rap_setup_reflector(struct rap_data *data);
 
 /* Adapter data management */
 static bool match_adapter(const void *data, const void *match_data)
@@ -188,13 +190,34 @@ static void rap_debug(const char *str, void *user_data)
 	DBG_IDX(0xffff, "%s", str);
 }
 
-static void rap_data_add(struct rap_data *data)
+static bool match_data(const void *data, const void *match_data)
+{
+	const struct rap_data *mdata = data;
+	const struct bt_rap *rap = match_data;
+
+	return mdata->rap == rap;
+}
+
+static bool match_device(const void *data, const void *match_data)
+{
+	const struct rap_data *mdata = data;
+	const struct btd_device *device = match_data;
+
+	return mdata->device == device;
+}
+
+static bool rap_data_add(struct rap_data *data)
 {
 	DBG("%p", data);
 
 	if (queue_find(sessions, NULL, data)) {
 		error("data %p already added", data);
-		return;
+		return false;
+	}
+
+	if (queue_find(sessions, match_device, data->device)) {
+		error("rap_data already exists for device %p", data->device);
+		return false;
 	}
 
 	bt_rap_set_debug(data->rap, rap_debug, NULL, NULL);
@@ -206,18 +229,19 @@ static void rap_data_add(struct rap_data *data)
 
 	if (data->service)
 		btd_service_set_user_data(data->service, data);
-}
-
-static bool match_data(const void *data, const void *match_data)
-{
-	const struct rap_data *mdata = data;
-	const struct bt_rap *rap = match_data;
 
-	return mdata->rap == rap;
+	return true;
 }
 
 static void rap_data_free(struct rap_data *data)
 {
+	if (data->dbus_registered) {
+		g_dbus_unregister_interface(btd_get_dbus_connection(),
+					    device_get_path(data->device),
+					    CS_INTERFACE);
+		data->dbus_registered = false;
+	}
+
 	if (data->service) {
 		btd_service_set_user_data(data->service, NULL);
 		bt_rap_set_user_data(data->rap, NULL);
@@ -281,31 +305,60 @@ static void rap_attached(struct bt_rap *rap, void *user_data)
 	struct rap_data *data;
 	struct bt_att *att;
 	struct btd_device *device;
+	int err;
 
 	DBG("%p", rap);
 
 	data = queue_find(sessions, match_data, rap);
-	if (data) {
-		DBG("data is already present");
-		return;
-	}
+	if (data)
+		goto setup;
 
 	att = bt_rap_get_att(rap);
 	if (!att) {
 		error("Unable to get att");
+		bt_rap_unref(rap);
 		return;
 	}
 
 	device = btd_adapter_find_device_by_fd(bt_att_get_fd(att));
 	if (!device) {
 		error("unable to find device");
+		bt_rap_unref(rap);
 		return;
 	}
 
+	/*
+	 * A separate bt_rap session for this device may already have a
+	 * rap_data (e.g. rap_probe()/rap_accept() ran first for the GATT
+	 * client role, while this "rap" came from bt_rap_get_session()'s
+	 * ad-hoc att-only session). Setting up a second HCI state machine
+	 * and D-Bus interface registration for the same device path would
+	 * race with the existing one, so rap_data_add() rejects it below
+	 * regardless of which side (ad-hoc vs. probe/accept) runs first.
+	 */
 	data = rap_data_new(device);
 	data->rap = rap;
 
-	rap_data_add(data);
+	if (!rap_data_add(data)) {
+		free(data);
+		bt_rap_unref(rap);
+		return;
+	}
+
+setup:
+	err = rap_setup_reflector(data);
+	if (err)
+		error("Failed to set up reflector session: %d", err);
+	/*
+	 * NOTE: on failure here, this rap_data is left half-constructed
+	 * (missing hci_sm and/or dbus_registered) with no retry. For a
+	 * session created via rap_probe()/rap_accept(), a later GATT client
+	 * idle event re-invokes rap_setup_reflector() and can still succeed.
+	 * But for an ad-hoc, client-less session (created above, from
+	 * bt_rap_get_session()), there is no ready/idle callback registered
+	 * anywhere to retry it, so a one-time failure here is permanent
+	 * until the ATT connection drops and rap_detached() cleans it up.
+	 */
 }
 
 enum cs_dict_target {
@@ -638,7 +691,11 @@ static int rap_probe(struct btd_service *service)
 		return -EINVAL;
 	}
 
-	rap_data_add(data);
+	if (!rap_data_add(data)) {
+		bt_rap_unref(data->rap);
+		free(data);
+		return -EINVAL;
+	}
 
 	data->ready_id = bt_rap_ready_register(data->rap, rap_ready, service,
 								NULL);
@@ -663,19 +720,13 @@ static void rap_remove(struct btd_service *service)
 		return;
 	}
 
-	g_dbus_unregister_interface(btd_get_dbus_connection(),
-				    device_get_path(device),
-				    CS_INTERFACE);
-
 	rap_data_remove(data);
 }
 
-static int rap_accept(struct btd_service *service)
+static int rap_setup_reflector(struct rap_data *data)
 {
-	struct btd_device *device = btd_service_get_device(service);
+	struct btd_device *device = data->device;
 	struct btd_adapter *adapter = device_get_adapter(device);
-	struct bt_gatt_client *client = btd_device_get_gatt_client(device);
-	struct rap_data *data = btd_service_get_user_data(service);
 	struct bt_att *att;
 	const bdaddr_t *bdaddr;
 	uint8_t bdaddr_type;
@@ -683,26 +734,18 @@ static int rap_accept(struct btd_service *service)
 	char addr[18];
 
 	ba2str(device_get_address(device), addr);
-	DBG("%s", addr);
 
-	if (!data) {
-		error("RAP Service not handled by profile");
-		return -EINVAL;
-	}
-
-	/* init shared adapter HCI channel */
-	if (!data->adapter_data) {
-		data->adapter_data = rap_adapter_data_ref(adapter);
+	if (!data->hci_sm) {
 		if (!data->adapter_data) {
-			error("Failed to get adapter HCI channel");
-			return -EINVAL;
+			data->adapter_data = rap_adapter_data_ref(adapter);
+			if (!data->adapter_data) {
+				error("Failed to get adapter HCI channel");
+				return -EINVAL;
+			}
+			DBG("Using shared HCI channel for adapter (ref=%d)",
+				data->adapter_data->ref_count);
 		}
-		DBG("Using shared HCI channel for adapter (ref_count=%d)",
-			data->adapter_data->ref_count);
-	}
 
-	/* per-device HCI state machine */
-	if (!data->hci_sm) {
 		data->hci_sm = bt_rap_attach_hci(data->rap,
 					data->adapter_data->hci,
 					btd_opts.defaults.bcs.role,
@@ -722,38 +765,74 @@ static int rap_accept(struct btd_service *service)
 					data);
 	}
 
-	if (!bt_rap_attach(data->rap, client)) {
-		error("RAP unable to attach");
-		return -EINVAL;
-	}
-
-	/* Set up connection handle mapping for CS event routing */
-	att = bt_rap_get_att(data->rap);
-	bdaddr = device_get_address(device);
-	bdaddr_type = device_get_le_address_type(device);
-
-	if (att && data->adapter_data->hci && data->hci_sm) {
-		if (bt_hci_get_conn_handle(data->adapter_data->hci,
-					(const uint8_t *) bdaddr, &handle)) {
-			DBG("Found conn handle 0x%04X for %s", handle, addr);
-			data->conn_handle = handle;
-			bt_rap_set_conn_hndl(data->hci_sm,
+	/* Set up connection handle mapping for CS event routing. Retried on
+	 * every call until it succeeds, since att may not be available yet
+	 * the first time this runs (e.g. called from bt_rap_attach()'s
+	 * attached callback before rap->client/att are set).
+	 */
+	if (!data->conn_handle) {
+		att = bt_rap_get_att(data->rap);
+		bdaddr = device_get_address(device);
+		bdaddr_type = device_get_le_address_type(device);
+
+		if (att && data->adapter_data->hci) {
+			if (bt_hci_get_conn_handle(data->adapter_data->hci,
+				(const uint8_t *)bdaddr, &handle)) {
+				DBG("conn handle 0x%04X for %s", handle, addr);
+				data->conn_handle = handle;
+				bt_rap_set_conn_hndl(data->hci_sm,
 					data->rap, handle,
-					(const uint8_t *) bdaddr,
+					(const uint8_t *)bdaddr,
 					bdaddr_type,
 					btd_device_is_initiator(device));
-		} else {
-			error("Failed to find connection handle for device %s",
-				addr);
+			} else {
+				error("Failed to find conn for device %s",
+					addr);
+			}
 		}
 	}
 
-	btd_service_connecting_complete(service, 0);
+	if (!data->dbus_registered) {
+		if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					device_get_path(device),
+					CS_INTERFACE, cs_dbus_methods,
+					NULL, cs_dbus_properties, data, NULL)) {
+			error("Failed to register %s interface for %s",
+				CS_INTERFACE, addr);
+			return -EINVAL;
+		}
+		data->dbus_registered = true;
+	}
+
+	return 0;
+}
 
-	g_dbus_register_interface(btd_get_dbus_connection(),
-				  device_get_path(data->device),
-				  CS_INTERFACE, cs_dbus_methods,
-				  NULL, cs_dbus_properties, data, NULL);
+static int rap_accept(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+	struct bt_gatt_client *client = btd_device_get_gatt_client(device);
+	struct rap_data *data = btd_service_get_user_data(service);
+	char addr[18];
+	int err;
+
+	ba2str(device_get_address(device), addr);
+	DBG("%s", addr);
+
+	if (!data) {
+		error("RAP Service not handled by profile");
+		return -EINVAL;
+	}
+
+	if (!bt_rap_attach(data->rap, client)) {
+		error("RAP unable to attach");
+		return -EINVAL;
+	}
+
+	err = rap_setup_reflector(data);
+	if (err)
+		return err;
+
+	btd_service_connecting_complete(service, 0);
 
 	return 0;
 }
@@ -814,7 +893,7 @@ static void rap_server_remove(struct btd_profile *p,
 static struct btd_profile rap_profile = {
 	.name		= "rap",
 	.priority	= BTD_PROFILE_PRIORITY_MEDIUM,
-	.remote_uuid	= GATT_UUID,
+	.remote_uuid	= RAS_UUID,
 	.local_uuid	= RAS_UUID,
 
 	.device_probe	= rap_probe,
--