This patchset implements GattManager1.UnregisterService(), and low-level
functions to remove service declarations (and its attributes) from the
attribute database.
Andre Guedes (1):
gatt: Add helper for removing GATT services
Claudio Takahasi (6):
gdbus: Avoid reporting GDBusClient disconnect twice
gatt: Rename external_app to external_service
gatt: Add GattManager1.UnregisterService()
gatt: Add removing service from the database
gatt: Fix possibly lost block when bluetoothd exits
doc: Add InvalidArguments to UnregisterService() errors
doc/gatt-api.txt | 3 +-
gdbus/client.c | 14 ++++-
src/gatt-dbus.c | 162 ++++++++++++++++++++++++++++++++++---------------------
src/gatt.c | 42 +++++++++++++++
src/gatt.h | 7 +++
5 files changed, 165 insertions(+), 63 deletions(-)
--
1.8.3.1
Hi Claudio,
On Thu, Apr 03, 2014, Claudio Takahasi wrote:
> This patchset implements GattManager1.UnregisterService(), and low-level
> functions to remove service declarations (and its attributes) from the
> attribute database.
>
> Changes v1-v0:
> * Fix potential invalid memory access
> * Cleanup of external service when registration fails
>
> Claudio Takahasi (3):
> gatt: Add removing service from the database
> gatt: Fix possibly lost block when bluetoothd exits
> doc: Add InvalidArguments to UnregisterService() errors
>
> doc/gatt-api.txt | 3 ++-
> src/gatt-dbus.c | 51 +++++++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 43 insertions(+), 11 deletions(-)
All three patches have been applied. Thanks.
Johan
This patch adds "org.bluez.Error.InvalidArguments" as possible error
returned by GattManager1.UnregisterService().
---
doc/gatt-api.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index deb519b..8c7975c 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -145,4 +145,5 @@ Methods RegisterService(object service, dict options)
must match the same value that has been used
on registration.
- Possible errors: org.bluez.Error.DoesNotExist
+ Possible errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.DoesNotExist
--
1.8.3.1
This patch fixes a possibly lost memory related to GDBusClient
and GDBusProxy objects when bluetoothd daemon exits and there
is registered external service.
==1503== by 0x47ECFB: g_dbus_client_new (client.c:1232)
==1503== by 0x461EC7: register_service (gatt-dbus.c:510)
---
src/gatt-dbus.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index d3fd717..26437e7 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -96,6 +96,19 @@ static gboolean external_service_destroy(void *user_data)
return FALSE;
}
+static void external_service_free(void *user_data)
+{
+ struct external_service *esvc = user_data;
+
+ /*
+ * Set callback to NULL to avoid potential race condition
+ * when calling remove_service and GDBusClient unref.
+ */
+ g_dbus_client_set_disconnect_watch(esvc->client, NULL, NULL);
+
+ external_service_destroy(user_data);
+}
+
static void remove_service(DBusConnection *conn, void *user_data)
{
struct external_service *esvc = user_data;
@@ -634,6 +647,8 @@ void gatt_dbus_manager_unregister(void)
g_hash_table_destroy(proxy_hash);
proxy_hash = NULL;
+ g_slist_free_full(external_services, external_service_free);
+
g_dbus_unregister_interface(btd_get_dbus_connection(), "/org/bluez",
GATT_MGR_IFACE);
}
--
1.8.3.1
This patch removes the service declaration and its attributes when
the external service implementation leaves the system bus, calls
UnregisterService(), or RegisterService() fails.
---
src/gatt-dbus.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 3ded9cd..d3fd717 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -56,6 +56,7 @@ struct external_service {
DBusMessage *reg;
GDBusClient *client;
GSList *proxies;
+ struct btd_attribute *service;
};
struct proxy_write_data {
@@ -99,10 +100,11 @@ static void remove_service(DBusConnection *conn, void *user_data)
{
struct external_service *esvc = user_data;
- /* TODO: Remove from the database */
-
external_services = g_slist_remove(external_services, esvc);
+ if (esvc->service)
+ btd_gatt_remove_service(esvc->service);
+
/*
* Do not run in the same loop, this may be a disconnect
* watch call and GDBusClient should not be destroyed.
@@ -336,7 +338,7 @@ static void proxy_write_cb(struct btd_attribute *attr,
}
-static int register_external_service(const struct external_service *esvc,
+static int register_external_service(struct external_service *esvc,
GDBusProxy *proxy)
{
DBusMessageIter iter;
@@ -360,7 +362,8 @@ static int register_external_service(const struct external_service *esvc,
if (bt_string_to_uuid(&uuid, str) < 0)
return -EINVAL;
- if (!btd_gatt_add_service(&uuid))
+ esvc->service = btd_gatt_add_service(&uuid);
+ if (!esvc->service)
return -EINVAL;
return 0;
@@ -481,18 +484,25 @@ static void client_ready(GDBusClient *client, void *user_data)
DBG("Added GATT service %s", esvc->path);
reply = dbus_message_new_method_return(esvc->reg);
- goto reply;
+ g_dbus_send_message(conn, reply);
+
+ dbus_message_unref(esvc->reg);
+ esvc->reg = NULL;
+
+ return;
fail:
error("Could not register external service: %s", esvc->path);
- reply = btd_error_invalid_args(esvc->reg);
- /* TODO: missing esvc/database cleanup */
+ /*
+ * Set callback to NULL to avoid potential race condition
+ * when calling remove_service and GDBusClient unref.
+ */
+ g_dbus_client_set_disconnect_watch(esvc->client, NULL, NULL);
-reply:
- dbus_message_unref(esvc->reg);
- esvc->reg = NULL;
+ remove_service(conn, esvc);
+ reply = btd_error_invalid_args(esvc->reg);
g_dbus_send_message(conn, reply);
}
@@ -578,6 +588,12 @@ static DBusMessage *unregister_service(DBusConnection *conn,
if (!strcmp(dbus_message_get_sender(msg), esvc->owner))
return btd_error_does_not_exist(msg);
+ /*
+ * Set callback to NULL to avoid potential race condition
+ * when calling remove_service and GDBusClient unref.
+ */
+ g_dbus_client_set_disconnect_watch(esvc->client, NULL, NULL);
+
remove_service(conn, esvc);
return dbus_message_new_method_return(msg);
--
1.8.3.1
This patchset implements GattManager1.UnregisterService(), and low-level
functions to remove service declarations (and its attributes) from the
attribute database.
Changes v1-v0:
* Fix potential invalid memory access
* Cleanup of external service when registration fails
Claudio Takahasi (3):
gatt: Add removing service from the database
gatt: Fix possibly lost block when bluetoothd exits
doc: Add InvalidArguments to UnregisterService() errors
doc/gatt-api.txt | 3 ++-
src/gatt-dbus.c | 51 +++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 43 insertions(+), 11 deletions(-)
--
1.8.3.1
Hi Johan:
On Thu, Apr 3, 2014 at 4:49 AM, Johan Hedberg <[email protected]> wrote:
> Hi Claudio,
>
> On Wed, Apr 02, 2014, Claudio Takahasi wrote:
>> This patch removes the service declaration and its attributes when
>> the external service implementation leaves the system bus, calls
>> UnregisterService(), or RegisterService() fails.
>> ---
>> src/gatt-dbus.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> I've applied patches 1-4.
>
>> @@ -99,7 +100,8 @@ static void remove_service(DBusConnection *conn, void *user_data)
>> {
>> struct external_service *esvc = user_data;
>>
>> - /* TODO: Remove from the database */
>> + if (esvc->service)
>> + btd_gatt_remove_service(esvc->service);
>
> Would it not be safer to set esvc->service to NULL after this?
>
>> @@ -360,7 +362,8 @@ static int register_external_service(const struct external_service *esvc,
>> if (bt_string_to_uuid(&uuid, str) < 0)
>> return -EINVAL;
>>
>> - if (!btd_gatt_add_service(&uuid))
>> + esvc->service = btd_gatt_add_service(&uuid);
>> + if (!esvc->service)
>> return -EINVAL;
>>
>> return 0;
>> @@ -487,7 +490,8 @@ fail:
>> error("Could not register external service: %s", esvc->path);
>>
>> reply = btd_error_invalid_args(esvc->reg);
>> - /* TODO: missing esvc/database cleanup */
>> + if (esvc->service)
>> + btd_gatt_remove_service(esvc->service);
>
> Same here.
>
> Johan
Actually, there is a potential invalid memory access related to esvc
(external_service), set esvc->service to NULL is not necessary.
Race condition & invalid memory access can happen when calling
remove_service callback & GDBusClient unref.
I will send another patchset fixing this problem.
Regards,
Claudio
Hi Claudio,
On Wed, Apr 02, 2014, Claudio Takahasi wrote:
> This patch removes the service declaration and its attributes when
> the external service implementation leaves the system bus, calls
> UnregisterService(), or RegisterService() fails.
> ---
> src/gatt-dbus.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
I've applied patches 1-4.
> @@ -99,7 +100,8 @@ static void remove_service(DBusConnection *conn, void *user_data)
> {
> struct external_service *esvc = user_data;
>
> - /* TODO: Remove from the database */
> + if (esvc->service)
> + btd_gatt_remove_service(esvc->service);
Would it not be safer to set esvc->service to NULL after this?
> @@ -360,7 +362,8 @@ static int register_external_service(const struct external_service *esvc,
> if (bt_string_to_uuid(&uuid, str) < 0)
> return -EINVAL;
>
> - if (!btd_gatt_add_service(&uuid))
> + esvc->service = btd_gatt_add_service(&uuid);
> + if (!esvc->service)
> return -EINVAL;
>
> return 0;
> @@ -487,7 +490,8 @@ fail:
> error("Could not register external service: %s", esvc->path);
>
> reply = btd_error_invalid_args(esvc->reg);
> - /* TODO: missing esvc/database cleanup */
> + if (esvc->service)
> + btd_gatt_remove_service(esvc->service);
Same here.
Johan
This patch adds GattManager1.UnregisterService() method implementation
according to doc/gatt-api.txt.
---
src/gatt-dbus.c | 64 ++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 17 deletions(-)
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index b29371d..3ded9cd 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -56,7 +56,6 @@ struct external_service {
DBusMessage *reg;
GDBusClient *client;
GSList *proxies;
- unsigned int watch;
};
struct proxy_write_data {
@@ -80,21 +79,35 @@ static int external_service_path_cmp(gconstpointer a, gconstpointer b)
return g_strcmp0(esvc->path, path);
}
-static void external_service_watch_destroy(gpointer user_data)
+static gboolean external_service_destroy(void *user_data)
{
struct external_service *esvc = user_data;
- /* TODO: Remove from the database */
-
- external_services = g_slist_remove(external_services, esvc);
-
g_dbus_client_unref(esvc->client);
+
if (esvc->reg)
dbus_message_unref(esvc->reg);
g_free(esvc->owner);
g_free(esvc->path);
g_free(esvc);
+
+ return FALSE;
+}
+
+static void remove_service(DBusConnection *conn, void *user_data)
+{
+ struct external_service *esvc = user_data;
+
+ /* TODO: Remove from the database */
+
+ external_services = g_slist_remove(external_services, esvc);
+
+ /*
+ * Do not run in the same loop, this may be a disconnect
+ * watch call and GDBusClient should not be destroyed.
+ */
+ g_idle_add(external_service_destroy, esvc);
}
static int proxy_path_cmp(gconstpointer a, gconstpointer b)
@@ -483,7 +496,7 @@ reply:
g_dbus_send_message(conn, reply);
}
-static struct external_service *new_external_service(DBusConnection *conn,
+static struct external_service *external_service_new(DBusConnection *conn,
DBusMessage *msg, const char *path)
{
struct external_service *esvc;
@@ -495,20 +508,13 @@ static struct external_service *new_external_service(DBusConnection *conn,
return NULL;
esvc = g_new0(struct external_service, 1);
-
- esvc->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
- sender, NULL, esvc, external_service_watch_destroy);
- if (esvc->watch == 0) {
- g_dbus_client_unref(client);
- g_free(esvc);
- return NULL;
- }
-
esvc->owner = g_strdup(sender);
esvc->reg = dbus_message_ref(msg);
esvc->client = client;
esvc->path = g_strdup(path);
+ g_dbus_client_set_disconnect_watch(client, remove_service, esvc);
+
g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
NULL, esvc);
@@ -536,7 +542,7 @@ static DBusMessage *register_service(DBusConnection *conn,
external_service_path_cmp))
return btd_error_already_exists(msg);
- esvc = new_external_service(conn, msg, path);
+ esvc = external_service_new(conn, msg, path);
if (!esvc)
return btd_error_failed(msg, "Not enough resources");
@@ -550,6 +556,30 @@ static DBusMessage *register_service(DBusConnection *conn,
static DBusMessage *unregister_service(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
+ struct external_service *esvc;
+ DBusMessageIter iter;
+ const char *path;
+ GSList *list;
+
+ if (!dbus_message_iter_init(msg, &iter))
+ return btd_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&iter, &path);
+
+ list = g_slist_find_custom(external_services, path,
+ external_service_path_cmp);
+ if (!list)
+ return btd_error_does_not_exist(msg);
+
+ esvc = list->data;
+ if (!strcmp(dbus_message_get_sender(msg), esvc->owner))
+ return btd_error_does_not_exist(msg);
+
+ remove_service(conn, esvc);
+
return dbus_message_new_method_return(msg);
}
--
1.8.3.1
This patch adds "org.bluez.Error.InvalidArguments" as possible error
returned by GattManager1.UnregisterService().
---
doc/gatt-api.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index deb519b..8c7975c 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -145,4 +145,5 @@ Methods RegisterService(object service, dict options)
must match the same value that has been used
on registration.
- Possible errors: org.bluez.Error.DoesNotExist
+ Possible errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.DoesNotExist
--
1.8.3.1
This patch fixes a possibly lost memory related to GDBusClient
and GDBusProxy objects when bluetoothd daemon exits and there
is registered external service.
==1503== by 0x47ECFB: g_dbus_client_new (client.c:1232)
==1503== by 0x461EC7: register_service (gatt-dbus.c:510)
---
src/gatt-dbus.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 17c3359..1a0e55e 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -96,6 +96,11 @@ static gboolean external_service_destroy(void *user_data)
return FALSE;
}
+static void external_service_free(void *user_data)
+{
+ external_service_destroy(user_data);
+}
+
static void remove_service(DBusConnection *conn, void *user_data)
{
struct external_service *esvc = user_data;
@@ -622,6 +627,8 @@ void gatt_dbus_manager_unregister(void)
g_hash_table_destroy(proxy_hash);
proxy_hash = NULL;
+ g_slist_free_full(external_services, external_service_free);
+
g_dbus_unregister_interface(btd_get_dbus_connection(), "/org/bluez",
GATT_MGR_IFACE);
}
--
1.8.3.1
This patch removes the service declaration and its attributes when
the external service implementation leaves the system bus, calls
UnregisterService(), or RegisterService() fails.
---
src/gatt-dbus.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 3ded9cd..17c3359 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -56,6 +56,7 @@ struct external_service {
DBusMessage *reg;
GDBusClient *client;
GSList *proxies;
+ struct btd_attribute *service;
};
struct proxy_write_data {
@@ -99,7 +100,8 @@ static void remove_service(DBusConnection *conn, void *user_data)
{
struct external_service *esvc = user_data;
- /* TODO: Remove from the database */
+ if (esvc->service)
+ btd_gatt_remove_service(esvc->service);
external_services = g_slist_remove(external_services, esvc);
@@ -336,7 +338,7 @@ static void proxy_write_cb(struct btd_attribute *attr,
}
-static int register_external_service(const struct external_service *esvc,
+static int register_external_service(struct external_service *esvc,
GDBusProxy *proxy)
{
DBusMessageIter iter;
@@ -360,7 +362,8 @@ static int register_external_service(const struct external_service *esvc,
if (bt_string_to_uuid(&uuid, str) < 0)
return -EINVAL;
- if (!btd_gatt_add_service(&uuid))
+ esvc->service = btd_gatt_add_service(&uuid);
+ if (!esvc->service)
return -EINVAL;
return 0;
@@ -487,7 +490,8 @@ fail:
error("Could not register external service: %s", esvc->path);
reply = btd_error_invalid_args(esvc->reg);
- /* TODO: missing esvc/database cleanup */
+ if (esvc->service)
+ btd_gatt_remove_service(esvc->service);
reply:
dbus_message_unref(esvc->reg);
--
1.8.3.1
Even for external applications providing multiple services, each
external service should have its own instance of GDBusClient. This
approach allows a more dynamic management of the registered services.
Based on this assumption, rename the external_app to external_service
is more logical.
---
src/gatt-dbus.c | 107 ++++++++++++++++++++++++++++----------------------------
1 file changed, 54 insertions(+), 53 deletions(-)
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 1bfa21f..b29371d 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -50,7 +50,7 @@
#define GATT_CHR_IFACE "org.bluez.GattCharacteristic1"
#define GATT_DESCRIPTOR_IFACE "org.bluez.GattDescriptor1"
-struct external_app {
+struct external_service {
char *owner;
char *path;
DBusMessage *reg;
@@ -70,31 +70,31 @@ struct proxy_write_data {
*/
static GHashTable *proxy_hash;
-static GSList *external_apps;
+static GSList *external_services;
-static int external_app_path_cmp(gconstpointer a, gconstpointer b)
+static int external_service_path_cmp(gconstpointer a, gconstpointer b)
{
- const struct external_app *eapp = a;
+ const struct external_service *esvc = a;
const char *path = b;
- return g_strcmp0(eapp->path, path);
+ return g_strcmp0(esvc->path, path);
}
-static void external_app_watch_destroy(gpointer user_data)
+static void external_service_watch_destroy(gpointer user_data)
{
- struct external_app *eapp = user_data;
+ struct external_service *esvc = user_data;
/* TODO: Remove from the database */
- external_apps = g_slist_remove(external_apps, eapp);
+ external_services = g_slist_remove(external_services, esvc);
- g_dbus_client_unref(eapp->client);
- if (eapp->reg)
- dbus_message_unref(eapp->reg);
+ g_dbus_client_unref(esvc->client);
+ if (esvc->reg)
+ dbus_message_unref(esvc->reg);
- g_free(eapp->owner);
- g_free(eapp->path);
- g_free(eapp);
+ g_free(esvc->owner);
+ g_free(esvc->path);
+ g_free(esvc);
}
static int proxy_path_cmp(gconstpointer a, gconstpointer b)
@@ -167,13 +167,13 @@ fail:
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- struct external_app *eapp = user_data;
+ struct external_service *esvc = user_data;
const char *interface, *path;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
- if (!g_str_has_prefix(path, eapp->path))
+ if (!g_str_has_prefix(path, esvc->path))
return;
if (g_strcmp0(interface, GATT_CHR_IFACE) != 0 &&
@@ -188,13 +188,13 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
* proxies sorted by path helps the logic to register the
* object path later.
*/
- eapp->proxies = g_slist_insert_sorted(eapp->proxies, proxy,
+ esvc->proxies = g_slist_insert_sorted(esvc->proxies, proxy,
proxy_path_cmp);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
{
- struct external_app *eapp = user_data;
+ struct external_service *esvc = user_data;
const char *interface, *path;
interface = g_dbus_proxy_get_interface(proxy);
@@ -202,7 +202,7 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
DBG("path %s iface %s", path, interface);
- eapp->proxies = g_slist_remove(eapp->proxies, proxy);
+ esvc->proxies = g_slist_remove(esvc->proxies, proxy);
}
static void proxy_read_cb(struct btd_attribute *attr,
@@ -323,7 +323,7 @@ static void proxy_write_cb(struct btd_attribute *attr,
}
-static int register_external_service(const struct external_app *eapp,
+static int register_external_service(const struct external_service *esvc,
GDBusProxy *proxy)
{
DBusMessageIter iter;
@@ -332,7 +332,7 @@ static int register_external_service(const struct external_app *eapp,
path = g_dbus_proxy_get_path(proxy);
iface = g_dbus_proxy_get_interface(proxy);
- if (g_strcmp0(eapp->path, path) != 0 ||
+ if (g_strcmp0(esvc->path, path) != 0 ||
g_strcmp0(iface, GATT_SERVICE_IFACE) != 0)
return -EINVAL;
@@ -450,43 +450,43 @@ static int register_external_characteristics(GSList *proxies)
static void client_ready(GDBusClient *client, void *user_data)
{
- struct external_app *eapp = user_data;
+ struct external_service *esvc = user_data;
GDBusProxy *proxy;
DBusConnection *conn = btd_get_dbus_connection();
DBusMessage *reply;
- if (!eapp->proxies)
+ if (!esvc->proxies)
goto fail;
- proxy = eapp->proxies->data;
- if (register_external_service(eapp, proxy) < 0)
+ proxy = esvc->proxies->data;
+ if (register_external_service(esvc, proxy) < 0)
goto fail;
- if (register_external_characteristics(g_slist_next(eapp->proxies)) < 0)
+ if (register_external_characteristics(g_slist_next(esvc->proxies)) < 0)
goto fail;
- DBG("Added GATT service %s", eapp->path);
+ DBG("Added GATT service %s", esvc->path);
- reply = dbus_message_new_method_return(eapp->reg);
+ reply = dbus_message_new_method_return(esvc->reg);
goto reply;
fail:
- error("Could not register external service: %s", eapp->path);
+ error("Could not register external service: %s", esvc->path);
- reply = btd_error_invalid_args(eapp->reg);
- /* TODO: missing eapp/database cleanup */
+ reply = btd_error_invalid_args(esvc->reg);
+ /* TODO: missing esvc/database cleanup */
reply:
- dbus_message_unref(eapp->reg);
- eapp->reg = NULL;
+ dbus_message_unref(esvc->reg);
+ esvc->reg = NULL;
g_dbus_send_message(conn, reply);
}
-static struct external_app *new_external_app(DBusConnection *conn,
+static struct external_service *new_external_service(DBusConnection *conn,
DBusMessage *msg, const char *path)
{
- struct external_app *eapp;
+ struct external_service *esvc;
GDBusClient *client;
const char *sender = dbus_message_get_sender(msg);
@@ -494,33 +494,33 @@ static struct external_app *new_external_app(DBusConnection *conn,
if (!client)
return NULL;
- eapp = g_new0(struct external_app, 1);
+ esvc = g_new0(struct external_service, 1);
- eapp->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
- sender, NULL, eapp, external_app_watch_destroy);
- if (eapp->watch == 0) {
+ esvc->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
+ sender, NULL, esvc, external_service_watch_destroy);
+ if (esvc->watch == 0) {
g_dbus_client_unref(client);
- g_free(eapp);
+ g_free(esvc);
return NULL;
}
- eapp->owner = g_strdup(sender);
- eapp->reg = dbus_message_ref(msg);
- eapp->client = client;
- eapp->path = g_strdup(path);
+ esvc->owner = g_strdup(sender);
+ esvc->reg = dbus_message_ref(msg);
+ esvc->client = client;
+ esvc->path = g_strdup(path);
g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
- NULL, eapp);
+ NULL, esvc);
- g_dbus_client_set_ready_watch(client, client_ready, eapp);
+ g_dbus_client_set_ready_watch(client, client_ready, esvc);
- return eapp;
+ return esvc;
}
static DBusMessage *register_service(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
- struct external_app *eapp;
+ struct external_service *esvc;
DBusMessageIter iter;
const char *path;
@@ -532,16 +532,17 @@ static DBusMessage *register_service(DBusConnection *conn,
dbus_message_iter_get_basic(&iter, &path);
- if (g_slist_find_custom(external_apps, path, external_app_path_cmp))
+ if (g_slist_find_custom(external_services, path,
+ external_service_path_cmp))
return btd_error_already_exists(msg);
- eapp = new_external_app(conn, msg, path);
- if (!eapp)
+ esvc = new_external_service(conn, msg, path);
+ if (!esvc)
return btd_error_failed(msg, "Not enough resources");
- external_apps = g_slist_prepend(external_apps, eapp);
+ external_services = g_slist_prepend(external_services, esvc);
- DBG("New app %p: %s", eapp, path);
+ DBG("New service %p: %s", esvc, path);
return NULL;
}
--
1.8.3.1
No matter if disconnection was reported previously, g_dbus_client_unref()
was always calling service disconnect callback. This patch fix the
following scenario:
1) service disconnects from the bus
2) disconnect callback gets called
3) client calls g_dbus_client_unref(), disconnect callback is called
again.
---
gdbus/client.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/gdbus/client.c b/gdbus/client.c
index 3bf883a..eb68a0f 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -51,6 +51,7 @@ struct GDBusClient {
GDBusWatchFunction connect_func;
void *connect_data;
GDBusWatchFunction disconn_func;
+ gboolean connected;
void *disconn_data;
GDBusMessageFunction signal_func;
void *signal_data;
@@ -1146,6 +1147,8 @@ static void service_connect(DBusConnection *conn, void *user_data)
get_managed_objects(client);
+ client->connected = TRUE;
+
g_dbus_client_unref(client);
}
@@ -1156,8 +1159,10 @@ static void service_disconnect(DBusConnection *conn, void *user_data)
g_list_free_full(client->proxy_list, proxy_free);
client->proxy_list = NULL;
- if (client->disconn_func)
+ if (client->disconn_func) {
client->disconn_func(conn, client->disconn_data);
+ client->connected = FALSE;
+ }
}
static DBusHandlerResult message_filter(DBusConnection *connection,
@@ -1210,6 +1215,7 @@ GDBusClient *g_dbus_client_new(DBusConnection *connection,
client->dbus_conn = dbus_connection_ref(connection);
client->service_name = g_strdup(service);
client->base_path = g_strdup(path);
+ client->connected = FALSE;
client->match_rules = g_ptr_array_sized_new(1);
g_ptr_array_set_free_func(client->match_rules, g_free);
@@ -1284,7 +1290,11 @@ void g_dbus_client_unref(GDBusClient *client)
g_list_free_full(client->proxy_list, proxy_free);
- if (client->disconn_func)
+ /*
+ * Don't call disconn_func twice if disconnection
+ * was previously reported.
+ */
+ if (client->disconn_func && client->connected)
client->disconn_func(client->dbus_conn, client->disconn_data);
g_dbus_remove_watch(client->dbus_conn, client->watch);
--
1.8.3.1
From: Andre Guedes <[email protected]>
This patch adds the btd_gatt_remove_service() helper which removes a GATT
primary (or secondary) Service declaration (including its characteristics)
from the local attribute database.
---
src/gatt.c | 42 ++++++++++++++++++++++++++++++++++++++++++
src/gatt.h | 7 +++++++
2 files changed, 49 insertions(+)
diff --git a/src/gatt.c b/src/gatt.c
index f07effa..3060462 100644
--- a/src/gatt.c
+++ b/src/gatt.c
@@ -26,6 +26,7 @@
#endif
#include <glib.h>
+#include <stdbool.h>
#include "log.h"
#include "lib/uuid.h"
@@ -104,6 +105,18 @@ static struct btd_attribute *new_attribute(const bt_uuid_t *type,
return attr;
}
+static bool is_service(const struct btd_attribute *attr)
+{
+ if (attr->type.type != BT_UUID16)
+ return false;
+
+ if (attr->type.value.u16 == GATT_PRIM_SVC_UUID ||
+ attr->type.value.u16 == GATT_SND_SVC_UUID)
+ return true;
+
+ return false;
+}
+
static int local_database_add(uint16_t handle, struct btd_attribute *attr)
{
attr->handle = handle;
@@ -149,6 +162,35 @@ struct btd_attribute *btd_gatt_add_service(const bt_uuid_t *uuid)
return attr;
}
+void btd_gatt_remove_service(struct btd_attribute *service)
+{
+ GList *list = g_list_find(local_attribute_db, service);
+ bool first_node;
+
+ if (!list)
+ return;
+
+ first_node = local_attribute_db == list;
+
+ /* Remove service declaration attribute */
+ free(list->data);
+ list = g_list_delete_link(list, list);
+
+ /* Remove all characteristics until next service declaration */
+ while (list && !is_service(list->data)) {
+ free(list->data);
+ list = g_list_delete_link(list, list);
+ }
+
+ /*
+ * When removing the first node, local attribute database head
+ * needs to be updated. Node removed from middle doesn't change
+ * the list head address.
+ */
+ if (first_node)
+ local_attribute_db = list;
+}
+
struct btd_attribute *btd_gatt_add_char(const bt_uuid_t *uuid,
uint8_t properties,
btd_attr_read_t read_cb,
diff --git a/src/gatt.h b/src/gatt.h
index da7af92..f16541e 100644
--- a/src/gatt.h
+++ b/src/gatt.h
@@ -81,6 +81,13 @@ typedef void (*btd_attr_write_t) (struct btd_attribute *attr,
struct btd_attribute *btd_gatt_add_service(const bt_uuid_t *uuid);
/*
+ * btd_gatt_remove_service - Remove a service (along with all its
+ * characteristics) from the local attribute database.
+ * @service: Service declaration attribute.
+ */
+void btd_gatt_remove_service(struct btd_attribute *service);
+
+/*
* btd_gatt_add_char - Add a characteristic (declaration and value attributes)
* to local attribute database.
* @uuid: Characteristic UUID (16-bits or 128-bits).
--
1.8.3.1