[PATCH 0/4] Remove deprecated fields from struct hci_dev

[PATCH 0/4] Remove deprecated fields from struct hci_dev

[Bruna Moreira]

These patches remove unused fields from struct hci_dev. The adapter name is
moved to struct btd_adapter. The entire struct hci_dev will be gone once the
device discovery logic is moved to the hciops/mgmtops plugin.

The last patch removes the read_local_version() from struct btd_adapter_ops. It
was being used only by code which was removed, but we left it in a separate
patch because we don't know if it is okay to remove it completely.

Note that no removed fields are used on D-Bus API nor internally.

Bruna Moreira (4):
  Remove ssp_mode from struct hci_dev
  Remove LMP/HCI version and manufacturer from struct hci_dev
  Move adapter name from struct hci_dev to struct btd_adapter
  Remove read_local_version() from struct btd_adapter_ops

 plugins/hciops.c  |   21 ---------------------
 plugins/mgmtops.c |   18 ------------------
 src/adapter.c     |   52 ++++++++++++++--------------------------------------
 src/adapter.h     |    9 ---------
 4 files changed, 14 insertions(+), 86 deletions(-)

[PATCH 1/4] Remove ssp_mode from struct hci_dev

[Bruna Moreira]

ssp_mode is duplicated information and is not being used anywhere.
---
 plugins/hciops.c |    9 ---------
 src/adapter.c    |    9 ---------
 src/adapter.h    |    3 ---
 3 files changed, 0 insertions(+), 21 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 93f6f21..7d2eca3 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -1658,7 +1658,6 @@ static void read_simple_pairing_mode_complete(int index, void *ptr)
 {
 	struct dev_info *dev = &devs[index];
 	read_simple_pairing_mode_rp *rp = ptr;
-	struct btd_adapter *adapter;
 
 	DBG("hci%d status %u", index, rp->status);
 
@@ -1667,14 +1666,6 @@ static void read_simple_pairing_mode_complete(int index, void *ptr)
 
 	dev->ssp_mode = rp->mode;
 	update_ext_inquiry_response(index);
-
-	adapter = manager_find_adapter(&dev->bdaddr);
-	if (!adapter) {
-		error("No matching adapter found");
-		return;
-	}
-
-	adapter_update_ssp_mode(adapter, rp->mode);
 }
 
 static void read_local_ext_features_complete(int index,
diff --git a/src/adapter.c b/src/adapter.c
index dbc2c93..06119dd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2623,15 +2623,6 @@ int btd_adapter_stop(struct btd_adapter *adapter)
 	return 0;
 }
 
-int adapter_update_ssp_mode(struct btd_adapter *adapter, uint8_t mode)
-{
-	struct hci_dev *dev = &adapter->dev;
-
-	dev->ssp_mode = mode;
-
-	return 0;
-}
-
 static void adapter_free(gpointer user_data)
 {
 	struct btd_adapter *adapter = user_data;
diff --git a/src/adapter.h b/src/adapter.h
index 308af75..14338b5 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -97,7 +97,6 @@ struct hci_dev {
 	uint16_t hci_rev;
 	uint16_t manufacturer;
 
-	uint8_t  ssp_mode;
 	char     name[MAX_NAME_LENGTH + 1];
 };
 
@@ -108,8 +107,6 @@ int btd_adapter_stop(struct btd_adapter *adapter);
 void btd_adapter_get_mode(struct btd_adapter *adapter, uint8_t *mode,
 					uint8_t *on_mode, gboolean *pairable);
 
-int adapter_update_ssp_mode(struct btd_adapter *adapter, uint8_t mode);
-
 struct btd_device *adapter_get_device(DBusConnection *conn,
 				struct btd_adapter *adapter, const char *address);
 
-- 
1.7.0.4

[PATCH 2/4] Remove LMP/HCI version and manufacturer from struct hci_dev

[Bruna Moreira]

These fields were not being used anywhere inside BlueZ.
---
 src/adapter.c |   13 -------------
 src/adapter.h |    4 ----
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 06119dd..6707241 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2675,7 +2675,6 @@ void btd_adapter_unref(struct btd_adapter *adapter)
 
 gboolean adapter_init(struct btd_adapter *adapter)
 {
-	struct hci_version ver;
 	struct hci_dev *dev;
 	int err;
 
@@ -2690,20 +2689,8 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	err = adapter_ops->read_local_version(adapter->dev_id, &ver);
-	if (err < 0) {
-		error("Can't read version info for hci%d: %s (%d)",
-					adapter->dev_id, strerror(-err), -err);
-		return FALSE;
-	}
-
 	dev = &adapter->dev;
 
-	dev->hci_rev = ver.hci_rev;
-	dev->lmp_ver = ver.lmp_ver;
-	dev->lmp_subver = ver.lmp_subver;
-	dev->manufacturer = ver.manufacturer;
-
 	err = adapter_ops->read_local_features(adapter->dev_id, dev->features);
 	if (err < 0) {
 		error("Can't read features for hci%d: %s (%d)",
diff --git a/src/adapter.h b/src/adapter.h
index 14338b5..01659f1 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -92,10 +92,6 @@ struct remote_dev_info {
 struct hci_dev {
 	uint8_t  features[8];
 	uint8_t  extfeatures[8];
-	uint8_t  lmp_ver;
-	uint16_t lmp_subver;
-	uint16_t hci_rev;
-	uint16_t manufacturer;
 
 	char     name[MAX_NAME_LENGTH + 1];
 };
-- 
1.7.0.4

[PATCH 3/4] Move adapter name from struct hci_dev to struct btd_adapter

[Bruna Moreira]

It makes more sense to have this information on the adapter structure,
as it is not HCI specific.
---
 src/adapter.c |   30 ++++++++++++++----------------
 src/adapter.h |    1 -
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 6707241..2b9f719 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -113,6 +113,7 @@ struct btd_adapter {
 	char *path;			/* adapter object path */
 	bdaddr_t bdaddr;		/* adapter Bluetooth Address */
 	uint32_t dev_class;		/* Class of Device */
+	char name[MAX_NAME_LENGTH + 1]; /* adapter name */
 	guint discov_timeout_id;	/* discoverable timeout id */
 	guint stop_discov_id;		/* stop inquiry/scanning id */
 	uint32_t discov_timeout;	/* discoverable time(sec) */
@@ -926,21 +927,19 @@ void btd_adapter_class_changed(struct btd_adapter *adapter, uint32_t new_class)
 
 void adapter_update_local_name(struct btd_adapter *adapter, const char *name)
 {
-	struct hci_dev *dev = &adapter->dev;
-
-	if (strncmp(name, dev->name, MAX_NAME_LENGTH) == 0)
+	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
 		return;
 
-	strncpy(dev->name, name, MAX_NAME_LENGTH);
+	strncpy(adapter->name, name, MAX_NAME_LENGTH);
 
 	if (main_opts.attrib_server)
 		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) dev->name, strlen(dev->name));
+			(const uint8_t *) adapter->name, strlen(adapter->name));
 
 	if (!adapter->name_stored) {
-		char *name_ptr = dev->name;
+		char *name_ptr = adapter->name;
 
-		write_local_name(&adapter->bdaddr, dev->name);
+		write_local_name(&adapter->bdaddr, adapter->name);
 
 		if (connection)
 			emit_property_changed(connection, adapter->path,
@@ -955,18 +954,17 @@ static DBusMessage *set_name(DBusConnection *conn, DBusMessage *msg,
 					const char *name, void *data)
 {
 	struct btd_adapter *adapter = data;
-	struct hci_dev *dev = &adapter->dev;
-	char *name_ptr = dev->name;
+	char *name_ptr = adapter->name;
 
 	if (!g_utf8_validate(name, -1, NULL)) {
 		error("Name change failed: supplied name isn't valid UTF-8");
 		return btd_error_invalid_args(msg);
 	}
 
-	if (strncmp(name, dev->name, MAX_NAME_LENGTH) == 0)
+	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
 		goto done;
 
-	strncpy(dev->name, name, MAX_NAME_LENGTH);
+	strncpy(adapter->name, name, MAX_NAME_LENGTH);
 	write_local_name(&adapter->bdaddr, name);
 	emit_property_changed(connection, adapter->path,
 					ADAPTER_INTERFACE, "Name",
@@ -1362,7 +1360,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
 
 	/* Name */
 	memset(str, 0, sizeof(str));
-	strncpy(str, (char *) adapter->dev.name, MAX_NAME_LENGTH);
+	strncpy(str, (char *) adapter->name, MAX_NAME_LENGTH);
 	property = str;
 
 	dict_append_entry(&dict, "Name", DBUS_TYPE_STRING, &property);
@@ -2454,7 +2452,7 @@ void btd_adapter_start(struct btd_adapter *adapter)
 	if (main_opts.le)
 		adapter_ops->enable_le(adapter->dev_id);
 
-	adapter_ops->set_name(adapter->dev_id, adapter->dev.name);
+	adapter_ops->set_name(adapter->dev_id, adapter->name);
 
 	if (read_local_class(&adapter->bdaddr, cls) < 0) {
 		uint32_t class = htobl(main_opts.class);
@@ -2698,13 +2696,13 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	if (read_local_name(&adapter->bdaddr, adapter->dev.name) < 0)
-		expand_name(adapter->dev.name, MAX_NAME_LENGTH, main_opts.name,
+	if (read_local_name(&adapter->bdaddr, adapter->name) < 0)
+		expand_name(adapter->name, MAX_NAME_LENGTH, main_opts.name,
 							adapter->dev_id);
 
 	if (main_opts.attrib_server)
 		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) dev->name, strlen(dev->name));
+			(const uint8_t *) adapter->name, strlen(adapter->name));
 
 	sdp_init_services_list(&adapter->bdaddr);
 	load_drivers(adapter);
diff --git a/src/adapter.h b/src/adapter.h
index 01659f1..4d8824e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -93,7 +93,6 @@ struct hci_dev {
 	uint8_t  features[8];
 	uint8_t  extfeatures[8];
 
-	char     name[MAX_NAME_LENGTH + 1];
 };
 
 void btd_adapter_start(struct btd_adapter *adapter);
-- 
1.7.0.4

[PATCH 4/4] Remove read_local_version() from struct btd_adapter_ops

[Bruna Moreira]

This callback is used only for getting HCI version/manufacturer
information, and is not being called anywhere in BlueZ anymore.
---
 plugins/hciops.c  |   12 ------------
 plugins/mgmtops.c |   18 ------------------
 src/adapter.h     |    1 -
 3 files changed, 0 insertions(+), 31 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 7d2eca3..2b9be3f 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -3231,17 +3231,6 @@ static int hciops_get_conn_list(int index, GSList **conns)
 	return 0;
 }
 
-static int hciops_read_local_version(int index, struct hci_version *ver)
-{
-	struct dev_info *dev = &devs[index];
-
-	DBG("hci%d", index);
-
-	memcpy(ver, &dev->ver, sizeof(*ver));
-
-	return 0;
-}
-
 static int hciops_read_local_features(int index, uint8_t *features)
 {
 	struct dev_info *dev = &devs[index];
@@ -3696,7 +3685,6 @@ static struct btd_adapter_ops hci_ops = {
 	.block_device = hciops_block_device,
 	.unblock_device = hciops_unblock_device,
 	.get_conn_list = hciops_get_conn_list,
-	.read_local_version = hciops_read_local_version,
 	.read_local_features = hciops_read_local_features,
 	.disconnect = hciops_disconnect,
 	.remove_bonding = hciops_remove_bonding,
diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index 042afc5..bb53e82 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -1706,23 +1706,6 @@ static int mgmt_get_conn_list(int index, GSList **conns)
 	return 0;
 }
 
-static int mgmt_read_local_version(int index, struct hci_version *ver)
-{
-	struct controller_info *info = &controllers[index];
-
-	DBG("index %d", index);
-
-	if (!info->valid)
-		return -ENODEV;
-
-	memset(ver, 0, sizeof(*ver));
-	ver->manufacturer = info->manufacturer;
-	ver->hci_ver = info->hci_ver;
-	ver->hci_rev = info->hci_rev;
-
-	return 0;
-}
-
 static int mgmt_read_local_features(int index, uint8_t *features)
 {
 	struct controller_info *info = &controllers[index];
@@ -2024,7 +2007,6 @@ static struct btd_adapter_ops mgmt_ops = {
 	.block_device = mgmt_block_device,
 	.unblock_device = mgmt_unblock_device,
 	.get_conn_list = mgmt_get_conn_list,
-	.read_local_version = mgmt_read_local_version,
 	.read_local_features = mgmt_read_local_features,
 	.disconnect = mgmt_disconnect,
 	.remove_bonding = mgmt_remove_bonding,
diff --git a/src/adapter.h b/src/adapter.h
index 4d8824e..ee3cae9 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -209,7 +209,6 @@ struct btd_adapter_ops {
 	int (*block_device) (int index, bdaddr_t *bdaddr);
 	int (*unblock_device) (int index, bdaddr_t *bdaddr);
 	int (*get_conn_list) (int index, GSList **conns);
-	int (*read_local_version) (int index, struct hci_version *ver);
 	int (*read_local_features) (int index, uint8_t *features);
 	int (*disconnect) (int index, bdaddr_t *bdaddr);
 	int (*remove_bonding) (int index, bdaddr_t *bdaddr);
-- 
1.7.0.4

Re: [PATCH 0/4] Remove deprecated fields from struct hci_dev

[Johan Hedberg]

Hi Bruna,

On Tue, Apr 19, 2011, Bruna Moreira wrote:
> These patches remove unused fields from struct hci_dev. The adapter name is
> moved to struct btd_adapter. The entire struct hci_dev will be gone once the
> device discovery logic is moved to the hciops/mgmtops plugin.
> 
> The last patch removes the read_local_version() from struct btd_adapter_ops. It
> was being used only by code which was removed, but we left it in a separate
> patch because we don't know if it is okay to remove it completely.
> 
> Note that no removed fields are used on D-Bus API nor internally.
> 
> Bruna Moreira (4):
>   Remove ssp_mode from struct hci_dev
>   Remove LMP/HCI version and manufacturer from struct hci_dev
>   Move adapter name from struct hci_dev to struct btd_adapter
>   Remove read_local_version() from struct btd_adapter_ops
> 
>  plugins/hciops.c  |   21 ---------------------
>  plugins/mgmtops.c |   18 ------------------
>  src/adapter.c     |   52 ++++++++++++++--------------------------------------
>  src/adapter.h     |    9 ---------
>  4 files changed, 14 insertions(+), 86 deletions(-)

All four patches have been pushed upstream. Thanks.

Johan