Re: [PATCH] gatt-database: Fix GATT object ordering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Olivier,

On Fri, Mar 4, 2016 at 12:55 AM, Olivier Martin <olivier@xxxxxxxxxxxx> wrote:
> There is no guarantee the objects returned by GetManagedObjects
> are ordered in the required order which is Service, Characteristic
> Descriptor due to their respective dependencies.
>
> This change ensures the objects are processed in the correct order.
> ---
>  src/gatt-database.c | 109 +++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 79 insertions(+), 30 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index d906e2b..dc020b8 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -89,6 +89,7 @@ struct gatt_app {
>         GDBusClient *client;
>         bool failed;
>         struct queue *services;
> +       struct queue *proxy_objects;

Call it proxies

>  };
>
>  struct external_service {
> @@ -360,11 +361,19 @@ static void service_free(void *data)
>         free(service);
>  }
>
> +static void proxy_object_free(void *data)
> +{
> +       struct GDBusProxy *proxy = data;
> +
> +       g_dbus_proxy_unref(proxy);
> +}
> +
>  static void app_free(void *data)
>  {
>         struct gatt_app *app = data;
>
>         queue_destroy(app->services, service_free);
> +       queue_destroy(app->proxy_objects, proxy_object_free);

I wouldn't even reference them, use a weak reference, in the first
place since they wont go away before ready callback.

>         if (app->client) {
>                 g_dbus_client_set_disconnect_watch(app->client, NULL, NULL);
> @@ -1428,39 +1437,12 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
>         if (app->failed)
>                 return;
>
> +       queue_push_tail(app->proxy_objects, g_dbus_proxy_ref(proxy));

Just push the without referencing here should be alright.

>         iface = g_dbus_proxy_get_interface(proxy);
>         path = g_dbus_proxy_get_path(proxy);
>
> -       if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
> -               struct external_service *service;
> -
> -               service = create_service(app, proxy, path);
> -               if (!service) {
> -                       app->failed = true;
> -                       return;
> -               }
> -       } else if (g_strcmp0(iface, GATT_CHRC_IFACE) == 0) {
> -               struct external_chrc *chrc;
> -
> -               chrc = chrc_create(app, proxy, path);
> -               if (!chrc) {
> -                       app->failed = true;
> -                       return;
> -               }
> -       } else if (g_strcmp0(iface, GATT_DESC_IFACE) == 0) {
> -               struct external_desc *desc;
> -
> -               desc = desc_create(app, proxy);
> -               if (!desc) {
> -                       app->failed = true;
> -                       return;
> -               }
> -       } else {
> -               DBG("Ignoring unrelated interface: %s", iface);
> -               return;
> -       }
> -
> -       DBG("Object added: path: %s, iface: %s", path, iface);
> +       DBG("Object received: %s, iface: %s", path, iface);
>  }
>
>  static void proxy_removed_cb(GDBusProxy *proxy, void *user_data)
> @@ -2139,12 +2121,78 @@ static bool database_add_app(struct gatt_app *app)
>         return true;
>  }
>
> +void register_service(void *data, void *user_data) {
> +       struct gatt_app *app = user_data;
> +       GDBusProxy *proxy = data;
> +        const char *iface = g_dbus_proxy_get_interface(proxy);
> +        const char *path = g_dbus_proxy_get_path(proxy);
> +
> +        if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
> +                struct external_service *service;
> +
> +                service = create_service(app, proxy, path);
> +                if (!service) {
> +                        app->failed = true;
> +                        return;
> +                }
> +        }
> +}
> +
> +void register_characteristic(void *data, void *user_data) {
> +        struct gatt_app *app = user_data;
> +        GDBusProxy *proxy = data;
> +        const char *iface = g_dbus_proxy_get_interface(proxy);
> +        const char *path = g_dbus_proxy_get_path(proxy);
> +
> +       if (g_strcmp0(iface, GATT_CHRC_IFACE) == 0) {
> +                struct external_chrc *chrc;
> +
> +                chrc = chrc_create(app, proxy, path);
> +                if (!chrc) {
> +                        app->failed = true;
> +                        return;
> +                }
> +        }
> +}
> +
> +void register_descriptor(void *data, void *user_data) {
> +        struct gatt_app *app = user_data;
> +        GDBusProxy *proxy = data;
> +        const char *iface = g_dbus_proxy_get_interface(proxy);
> +        const char *path = g_dbus_proxy_get_path(proxy);
> +
> +       if (g_strcmp0(iface, GATT_DESC_IFACE) == 0) {
> +                struct external_desc *desc;
> +
> +                desc = desc_create(app, proxy);
> +                if (!desc) {
> +                        app->failed = true;
> +                        return;
> +                }
> +        }
> +}
> +
>  static void client_ready_cb(GDBusClient *client, void *user_data)
>  {
>         struct gatt_app *app = user_data;
>         DBusMessage *reply;
>         bool fail = false;
>
> +       /*
> +        * Process received objects
> +        */
> +       if (queue_isempty(app->proxy_objects)) {
> +               error("No object received");
> +                fail = true;
> +                reply = btd_error_failed(app->reg,
> +                                        "No object received");
> +                goto reply;
> +       }
> +
> +       queue_foreach(app->proxy_objects, register_service, app);
> +       queue_foreach(app->proxy_objects, register_characteristic, app);
> +       queue_foreach(app->proxy_objects, register_descriptor, app);
> +
>         if (!app->services || app->failed) {
>                 error("No valid external GATT objects found");
>                 fail = true;
> @@ -2198,6 +2246,7 @@ static struct gatt_app *create_app(DBusConnection *conn, DBusMessage *msg,
>                 goto fail;
>
>         app->services = queue_new();
> +       app->proxy_objects = queue_new();
>         app->reg = dbus_message_ref(msg);
>
>         g_dbus_client_set_disconnect_watch(app->client, client_disconnect_cb,
> --
> 2.1.4

I could fix most of the comments above but there is way too many
coding style problems:

ERROR:CODE_INDENT: code indent should use tabs where possible
#83: FILE: src/gatt-database.c:2127:
+        const char *iface = g_dbus_proxy_get_interface(proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#83: FILE: src/gatt-database.c:2127:
+        const char *iface = g_dbus_proxy_get_interface(proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#84: FILE: src/gatt-database.c:2128:
+        const char *path = g_dbus_proxy_get_path(proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#84: FILE: src/gatt-database.c:2128:
+        const char *path = g_dbus_proxy_get_path(proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#86: FILE: src/gatt-database.c:2130:
+        if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#86: FILE: src/gatt-database.c:2130:
+        if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {$

ERROR:CODE_INDENT: code indent should use tabs where possible
#87: FILE: src/gatt-database.c:2131:
+                struct external_service *service;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#87: FILE: src/gatt-database.c:2131:
+                struct external_service *service;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#89: FILE: src/gatt-database.c:2133:
+                service = create_service(app, proxy, path);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#89: FILE: src/gatt-database.c:2133:
+                service = create_service(app, proxy, path);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#90: FILE: src/gatt-database.c:2134:
+                if (!service) {$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#90: FILE: src/gatt-database.c:2134:
+                if (!service) {$

ERROR:CODE_INDENT: code indent should use tabs where possible
#91: FILE: src/gatt-database.c:2135:
+                        app->failed = true;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#91: FILE: src/gatt-database.c:2135:
+                        app->failed = true;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#92: FILE: src/gatt-database.c:2136:
+                        return;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#92: FILE: src/gatt-database.c:2136:
+                        return;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#93: FILE: src/gatt-database.c:2137:
+                }$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#93: FILE: src/gatt-database.c:2137:
+                }$

ERROR:CODE_INDENT: code indent should use tabs where possible
#94: FILE: src/gatt-database.c:2138:
+        }$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#94: FILE: src/gatt-database.c:2138:
+        }$

ERROR:CODE_INDENT: code indent should use tabs where possible
#98: FILE: src/gatt-database.c:2142:
+        struct gatt_app *app = user_data;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#98: FILE: src/gatt-database.c:2142:
+        struct gatt_app *app = user_data;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#99: FILE: src/gatt-database.c:2143:
+        GDBusProxy *proxy = data;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#99: FILE: src/gatt-database.c:2143:
+        GDBusProxy *proxy = data;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#100: FILE: src/gatt-database.c:2144:
+        const char *iface = g_dbus_proxy_get_interface(proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#100: FILE: src/gatt-database.c:2144:
+        const char *iface = g_dbus_proxy_get_interface(proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#101: FILE: src/gatt-database.c:2145:
+        const char *path = g_dbus_proxy_get_path(proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#101: FILE: src/gatt-database.c:2145:
+        const char *path = g_dbus_proxy_get_path(proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#104: FILE: src/gatt-database.c:2148:
+                struct external_chrc *chrc;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#104: FILE: src/gatt-database.c:2148:
+                struct external_chrc *chrc;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#106: FILE: src/gatt-database.c:2150:
+                chrc = chrc_create(app, proxy, path);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#106: FILE: src/gatt-database.c:2150:
+                chrc = chrc_create(app, proxy, path);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#107: FILE: src/gatt-database.c:2151:
+                if (!chrc) {$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#107: FILE: src/gatt-database.c:2151:
+                if (!chrc) {$

ERROR:CODE_INDENT: code indent should use tabs where possible
#108: FILE: src/gatt-database.c:2152:
+                        app->failed = true;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#108: FILE: src/gatt-database.c:2152:
+                        app->failed = true;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#109: FILE: src/gatt-database.c:2153:
+                        return;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#109: FILE: src/gatt-database.c:2153:
+                        return;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#110: FILE: src/gatt-database.c:2154:
+                }$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#110: FILE: src/gatt-database.c:2154:
+                }$

ERROR:CODE_INDENT: code indent should use tabs where possible
#111: FILE: src/gatt-database.c:2155:
+        }$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#111: FILE: src/gatt-database.c:2155:
+        }$

ERROR:CODE_INDENT: code indent should use tabs where possible
#115: FILE: src/gatt-database.c:2159:
+        struct gatt_app *app = user_data;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#115: FILE: src/gatt-database.c:2159:
+        struct gatt_app *app = user_data;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#116: FILE: src/gatt-database.c:2160:
+        GDBusProxy *proxy = data;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#116: FILE: src/gatt-database.c:2160:
+        GDBusProxy *proxy = data;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#117: FILE: src/gatt-database.c:2161:
+        const char *iface = g_dbus_proxy_get_interface(proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#117: FILE: src/gatt-database.c:2161:
+        const char *iface = g_dbus_proxy_get_interface(proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#118: FILE: src/gatt-database.c:2162:
+        const char *path = g_dbus_proxy_get_path(proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#118: FILE: src/gatt-database.c:2162:
+        const char *path = g_dbus_proxy_get_path(proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#121: FILE: src/gatt-database.c:2165:
+                struct external_desc *desc;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#121: FILE: src/gatt-database.c:2165:
+                struct external_desc *desc;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#123: FILE: src/gatt-database.c:2167:
+                desc = desc_create(app, proxy);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#123: FILE: src/gatt-database.c:2167:
+                desc = desc_create(app, proxy);$

ERROR:CODE_INDENT: code indent should use tabs where possible
#124: FILE: src/gatt-database.c:2168:
+                if (!desc) {$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#124: FILE: src/gatt-database.c:2168:
+                if (!desc) {$

ERROR:CODE_INDENT: code indent should use tabs where possible
#125: FILE: src/gatt-database.c:2169:
+                        app->failed = true;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#125: FILE: src/gatt-database.c:2169:
+                        app->failed = true;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#126: FILE: src/gatt-database.c:2170:
+                        return;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#126: FILE: src/gatt-database.c:2170:
+                        return;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#127: FILE: src/gatt-database.c:2171:
+                }$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#127: FILE: src/gatt-database.c:2171:
+                }$

ERROR:CODE_INDENT: code indent should use tabs where possible
#128: FILE: src/gatt-database.c:2172:
+        }$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#128: FILE: src/gatt-database.c:2172:
+        }$

ERROR:CODE_INDENT: code indent should use tabs where possible
#142: FILE: src/gatt-database.c:2186:
+                fail = true;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#142: FILE: src/gatt-database.c:2186:
+                fail = true;$

ERROR:CODE_INDENT: code indent should use tabs where possible
#143: FILE: src/gatt-database.c:2187:
+                reply = btd_error_failed(app->reg,$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#143: FILE: src/gatt-database.c:2187:
+                reply = btd_error_failed(app->reg,$

ERROR:CODE_INDENT: code indent should use tabs where possible
#144: FILE: src/gatt-database.c:2188:
+                                        "No object received");$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#144: FILE: src/gatt-database.c:2188:
+                                        "No object received");$

ERROR:CODE_INDENT: code indent should use tabs where possible
#145: FILE: src/gatt-database.c:2189:
+                goto reply;$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#145: FILE: src/gatt-database.c:2189:
+                goto reply;$



-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux