Message ID | 20210722152159.Bluez.v2.8.I517e5199ac8019b770c7ee8c92a294ec1c752748@changeid (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Hi manintainers, | expand |
Hi Howard, On Thu, Jul 22, 2021 at 12:23 AM Howard Chung <howardchung@google.com> wrote: > > From: Yun-Hao Chung <howardchung@chromium.org> > > This adds an D-BUS client to listen for DeviceAdd and DeviceRemove. It > is necessary for implementation of "AffectedByPolicy" property since it > needs to register an interface for each device object and unregister it > once the device gets removed. > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org> > --- > The following test steps were performed: > 1. start discovery using UI > 2. verify device_data were added by checking system log > 3. stop discovery > 4. verify device_data were removed after a few seconds by checking > system log > > (no changes since v1) > > plugins/admin_policy.c | 154 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 150 insertions(+), 4 deletions(-) > > diff --git a/plugins/admin_policy.c b/plugins/admin_policy.c > index 270d42366cd6..73d695ef976b 100644 > --- a/plugins/admin_policy.c > +++ b/plugins/admin_policy.c > @@ -20,6 +20,7 @@ > > #include "src/adapter.h" > #include "src/dbus-common.h" > +#include "src/device.h" > #include "src/error.h" > #include "src/log.h" > #include "src/plugin.h" > @@ -29,7 +30,12 @@ > #define ADMIN_POLICY_SET_INTERFACE "org.bluez.AdminPolicySet1" > #define ADMIN_POLICY_STATUS_INTERFACE "org.bluez.AdminPolicyStatus1" > > +#define DBUS_BLUEZ_SERVICE "org.bluez" > +#define BTD_DEVICE_INTERFACE "org.bluez.Device1" > + > static DBusConnection *dbus_conn; > +static GDBusClient *dbus_client; > +static struct queue *devices; /* List of struct device_data objects */ > > /* |policy_data| has the same life cycle as btd_adapter */ > static struct btd_admin_policy { > @@ -38,6 +44,11 @@ static struct btd_admin_policy { > struct queue *service_allowlist; > } *policy_data = NULL; > > +struct device_data { > + struct btd_device *device; > + char *path; > +}; > + > static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter) > { > struct btd_admin_policy *admin_policy = NULL; > @@ -203,8 +214,122 @@ static const GDBusPropertyTable admin_policy_adapter_properties[] = { > { } > }; > > +static bool device_data_match(const void *a, const void *b) > +{ > + const struct device_data *data = a; > + const struct btd_device *dev = b; > + > + if (!data) { > + error("Unexpected NULL device_data"); > + return false; > + } > + > + return data->device == dev; > +} > + > +static bool device_data_match_by_path(const void *a, const void *b) > +{ > + const struct device_data *data = a; > + const char *path = b; > + > + if (!data) { > + error("Unexpected NULL device_data"); > + return false; > + } > + > + return strcmp(data->path, b) == 0; > +} > + > +static void free_device_data(void *data) > +{ > + struct device_data *device_data = data; > + > + g_free(device_data->path); > + g_free(device_data); > +} > + > +static void remove_device_data(void *data) > +{ > + struct device_data *device_data = data; > + > + DBG("device_data for %s removing", device_data->path); > + > + queue_remove(devices, device_data); > + free_device_data(device_data); > +} > + > +static void add_device_data(struct btd_device *device) > +{ > + struct btd_adapter *adapter = device_get_adapter(device); > + struct device_data *data; > + > + if (queue_find(devices, device_data_match, device)) > + return; > + > + data = g_new0(struct device_data, 1); > + if (!data) { > + btd_error(btd_adapter_get_index(adapter), > + "Failed to allocate memory for device_data"); > + return; > + } > + > + data->device = device; > + data->path = g_strdup(device_get_path(device)); > + queue_push_tail(devices, data); > + > + DBG("device_data for %s added", data->path); > +} > + > +static struct btd_device *find_device_by_proxy(GDBusProxy *proxy) > +{ > + const char *path = g_dbus_proxy_get_path(proxy); > + const char *iface = g_dbus_proxy_get_interface(proxy); > + struct btd_device *device; > + > + if (strcmp(iface, BTD_DEVICE_INTERFACE) != 0) > + return NULL; > + > + device = btd_adapter_find_device_by_path(policy_data->adapter, path); > + > + if (!device) { > + btd_warn(adapter_get_path(policy_data->adapter), > + "Device path %s is not found", path); > + } > + > + return device; > +} > + > +static void object_added_cb(GDBusProxy *proxy, void *user_data) > +{ > + struct btd_device *device; > + > + device = find_device_by_proxy(proxy); > + > + if (!device) > + return; > + > + add_device_data(device); > +} > + > +static void object_removed_cb(GDBusProxy *proxy, void *user_data) > +{ > + const char *path = g_dbus_proxy_get_path(proxy); > + const char *iface = g_dbus_proxy_get_interface(proxy); > + struct device_data *data; > + > + if (strcmp(iface, BTD_DEVICE_INTERFACE) != 0) > + return; > + > + data = queue_find(devices, device_data_match_by_path, path); > + > + if (data) > + remove_device_data(data); > +} > + > static int admin_policy_adapter_probe(struct btd_adapter *adapter) > { > + const char *adapter_path; > + > if (policy_data) { > btd_warn(policy_data->adapter_id, > "Policy data already exists"); > @@ -216,33 +341,43 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter) > if (!policy_data) > return -ENOMEM; > > - if (!g_dbus_register_interface(dbus_conn, adapter_get_path(adapter), > + adapter_path = adapter_get_path(adapter); > + > + if (!g_dbus_register_interface(dbus_conn, adapter_path, > ADMIN_POLICY_SET_INTERFACE, > admin_policy_adapter_methods, NULL, > NULL, policy_data, admin_policy_free)) { > btd_error(policy_data->adapter_id, > "Admin Policy Set interface init failed on path %s", > - adapter_get_path(adapter)); > + adapter_path); > return -EINVAL; > } > > btd_info(policy_data->adapter_id, > "Admin Policy Set interface registered"); > > - if (!g_dbus_register_interface(dbus_conn, adapter_get_path(adapter), > + if (!g_dbus_register_interface(dbus_conn, adapter_path, > ADMIN_POLICY_STATUS_INTERFACE, > NULL, NULL, > admin_policy_adapter_properties, > policy_data, admin_policy_free)) { > btd_error(policy_data->adapter_id, > "Admin Policy Status interface init failed on path %s", > - adapter_get_path(adapter)); > + adapter_path); > return -EINVAL; > } > > btd_info(policy_data->adapter_id, > "Admin Policy Status interface registered"); > > + dbus_client = g_dbus_client_new(dbus_conn, DBUS_BLUEZ_SERVICE, > + adapter_path); > + > + g_dbus_client_set_proxy_handlers(dbus_client, object_added_cb, > + object_removed_cb, NULL, NULL); > + > + g_dbus_client_set_ready_watch(dbus_client, NULL, NULL); I hope it wasn't one of my comments that led you to do this, because it is really a bad idea to listen to our own signals like that since it comes from the same process it just adds a round trip to the D-Bus daemon for no reason. Perhaps we could extend the btd_adapter_driver to have device_added/device_remove callbacks so whenever a new device is added or removed the driver will get notified. > + > return 0; > } > > @@ -257,6 +392,7 @@ static int admin_policy_init(void) > DBG(""); > > dbus_conn = btd_get_dbus_connection(); > + devices = queue_new(); > > return btd_register_adapter_driver(&admin_policy_driver); > } > @@ -266,9 +402,19 @@ static void admin_policy_exit(void) > DBG(""); > > btd_unregister_adapter_driver(&admin_policy_driver); > + queue_destroy(devices, free_device_data); > > if (policy_data) > admin_policy_free(policy_data); > + > + if (dbus_client) { > + g_dbus_client_set_disconnect_watch(dbus_client, NULL, NULL); > + g_dbus_client_set_proxy_handlers(dbus_client, NULL, NULL, NULL, > + NULL); > + g_dbus_client_set_ready_watch(dbus_client, NULL, NULL); > + g_dbus_client_unref(dbus_client); > + dbus_client = NULL; > + } > } > > BLUETOOTH_PLUGIN_DEFINE(admin_policy, VERSION, > -- > 2.32.0.402.g57bb445576-goog >
diff --git a/plugins/admin_policy.c b/plugins/admin_policy.c index 270d42366cd6..73d695ef976b 100644 --- a/plugins/admin_policy.c +++ b/plugins/admin_policy.c @@ -20,6 +20,7 @@ #include "src/adapter.h" #include "src/dbus-common.h" +#include "src/device.h" #include "src/error.h" #include "src/log.h" #include "src/plugin.h" @@ -29,7 +30,12 @@ #define ADMIN_POLICY_SET_INTERFACE "org.bluez.AdminPolicySet1" #define ADMIN_POLICY_STATUS_INTERFACE "org.bluez.AdminPolicyStatus1" +#define DBUS_BLUEZ_SERVICE "org.bluez" +#define BTD_DEVICE_INTERFACE "org.bluez.Device1" + static DBusConnection *dbus_conn; +static GDBusClient *dbus_client; +static struct queue *devices; /* List of struct device_data objects */ /* |policy_data| has the same life cycle as btd_adapter */ static struct btd_admin_policy { @@ -38,6 +44,11 @@ static struct btd_admin_policy { struct queue *service_allowlist; } *policy_data = NULL; +struct device_data { + struct btd_device *device; + char *path; +}; + static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter) { struct btd_admin_policy *admin_policy = NULL; @@ -203,8 +214,122 @@ static const GDBusPropertyTable admin_policy_adapter_properties[] = { { } }; +static bool device_data_match(const void *a, const void *b) +{ + const struct device_data *data = a; + const struct btd_device *dev = b; + + if (!data) { + error("Unexpected NULL device_data"); + return false; + } + + return data->device == dev; +} + +static bool device_data_match_by_path(const void *a, const void *b) +{ + const struct device_data *data = a; + const char *path = b; + + if (!data) { + error("Unexpected NULL device_data"); + return false; + } + + return strcmp(data->path, b) == 0; +} + +static void free_device_data(void *data) +{ + struct device_data *device_data = data; + + g_free(device_data->path); + g_free(device_data); +} + +static void remove_device_data(void *data) +{ + struct device_data *device_data = data; + + DBG("device_data for %s removing", device_data->path); + + queue_remove(devices, device_data); + free_device_data(device_data); +} + +static void add_device_data(struct btd_device *device) +{ + struct btd_adapter *adapter = device_get_adapter(device); + struct device_data *data; + + if (queue_find(devices, device_data_match, device)) + return; + + data = g_new0(struct device_data, 1); + if (!data) { + btd_error(btd_adapter_get_index(adapter), + "Failed to allocate memory for device_data"); + return; + } + + data->device = device; + data->path = g_strdup(device_get_path(device)); + queue_push_tail(devices, data); + + DBG("device_data for %s added", data->path); +} + +static struct btd_device *find_device_by_proxy(GDBusProxy *proxy) +{ + const char *path = g_dbus_proxy_get_path(proxy); + const char *iface = g_dbus_proxy_get_interface(proxy); + struct btd_device *device; + + if (strcmp(iface, BTD_DEVICE_INTERFACE) != 0) + return NULL; + + device = btd_adapter_find_device_by_path(policy_data->adapter, path); + + if (!device) { + btd_warn(adapter_get_path(policy_data->adapter), + "Device path %s is not found", path); + } + + return device; +} + +static void object_added_cb(GDBusProxy *proxy, void *user_data) +{ + struct btd_device *device; + + device = find_device_by_proxy(proxy); + + if (!device) + return; + + add_device_data(device); +} + +static void object_removed_cb(GDBusProxy *proxy, void *user_data) +{ + const char *path = g_dbus_proxy_get_path(proxy); + const char *iface = g_dbus_proxy_get_interface(proxy); + struct device_data *data; + + if (strcmp(iface, BTD_DEVICE_INTERFACE) != 0) + return; + + data = queue_find(devices, device_data_match_by_path, path); + + if (data) + remove_device_data(data); +} + static int admin_policy_adapter_probe(struct btd_adapter *adapter) { + const char *adapter_path; + if (policy_data) { btd_warn(policy_data->adapter_id, "Policy data already exists"); @@ -216,33 +341,43 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter) if (!policy_data) return -ENOMEM; - if (!g_dbus_register_interface(dbus_conn, adapter_get_path(adapter), + adapter_path = adapter_get_path(adapter); + + if (!g_dbus_register_interface(dbus_conn, adapter_path, ADMIN_POLICY_SET_INTERFACE, admin_policy_adapter_methods, NULL, NULL, policy_data, admin_policy_free)) { btd_error(policy_data->adapter_id, "Admin Policy Set interface init failed on path %s", - adapter_get_path(adapter)); + adapter_path); return -EINVAL; } btd_info(policy_data->adapter_id, "Admin Policy Set interface registered"); - if (!g_dbus_register_interface(dbus_conn, adapter_get_path(adapter), + if (!g_dbus_register_interface(dbus_conn, adapter_path, ADMIN_POLICY_STATUS_INTERFACE, NULL, NULL, admin_policy_adapter_properties, policy_data, admin_policy_free)) { btd_error(policy_data->adapter_id, "Admin Policy Status interface init failed on path %s", - adapter_get_path(adapter)); + adapter_path); return -EINVAL; } btd_info(policy_data->adapter_id, "Admin Policy Status interface registered"); + dbus_client = g_dbus_client_new(dbus_conn, DBUS_BLUEZ_SERVICE, + adapter_path); + + g_dbus_client_set_proxy_handlers(dbus_client, object_added_cb, + object_removed_cb, NULL, NULL); + + g_dbus_client_set_ready_watch(dbus_client, NULL, NULL); + return 0; } @@ -257,6 +392,7 @@ static int admin_policy_init(void) DBG(""); dbus_conn = btd_get_dbus_connection(); + devices = queue_new(); return btd_register_adapter_driver(&admin_policy_driver); } @@ -266,9 +402,19 @@ static void admin_policy_exit(void) DBG(""); btd_unregister_adapter_driver(&admin_policy_driver); + queue_destroy(devices, free_device_data); if (policy_data) admin_policy_free(policy_data); + + if (dbus_client) { + g_dbus_client_set_disconnect_watch(dbus_client, NULL, NULL); + g_dbus_client_set_proxy_handlers(dbus_client, NULL, NULL, NULL, + NULL); + g_dbus_client_set_ready_watch(dbus_client, NULL, NULL); + g_dbus_client_unref(dbus_client); + dbus_client = NULL; + } } BLUETOOTH_PLUGIN_DEFINE(admin_policy, VERSION,