Re: [PATCH] Update SDP storage records when a record is deleted.

Jaikumar Ganesh
2009-10-23T15:17:51Z

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

Hi Luiz:

On Fri, Oct 23, 2009 at 6:49 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi,
>
> On Thu, Oct 22, 2009 at 8:19 PM, Jaikumar Ganesh <jaikumar@google.com> wrote:
> >        for (list = uuids; list; list = list->next)
> >                device->uuids = g_slist_remove(device->uuids, list->data);
>
> It looks like to me this would be the proper place to remove the
> records from the storage, since we are updating the uuids list there
> is no big deal to redo that in another function.
>
> So it would look like this:
>
> diff --git a/src/device.c b/src/device.c
> index 6cb9641..708e069 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1134,8 +1134,6 @@ static void device_remove_drivers(struct
> btd_device *device, GSList *uuids)
>                next = list->next;
>
>                for (uuid = driver->uuids; *uuid; uuid++) {
> -                       sdp_record_t *rec;
> -
>                        if (!g_slist_find_custom(uuids, *uuid,
>                                        (GCompareFunc) strcasecmp))
>                                continue;
> @@ -1148,15 +1146,6 @@ static void device_remove_drivers(struct
> btd_device *device, GSList *uuids)
>                                                                driver_data);
>                        g_free(driver_data);
>
> -                       rec = find_record_in_list(records, *uuid);
> -                       if (!rec)
> -                               break;
> -
> -                       delete_record(srcaddr, dstaddr, rec->handle);
> -
> -                       records = sdp_list_remove(records, rec);
> -                       sdp_record_free(rec);
> -
>                        break;
>                }
>        }
> @@ -1164,8 +1153,19 @@ static void device_remove_drivers(struct
> btd_device *device, GSList *uuids)
>        if (records)
>                sdp_list_free(records, (sdp_free_func_t) sdp_record_free);
>
> -       for (list = uuids; list; list = list->next)
> +       for (list = uuids; list; list = list->next) {
> +               sdp_record_t *rec;
> +
>                device->uuids = g_slist_remove(device->uuids, list->data);
> +               rec = find_record_in_list(records, list->data);
> +               if (!rec)
> +                       break;
> +
> +               delete_record(srcaddr, dstaddr, rec->handle);
> +
> +               records = sdp_list_remove(records, rec);
> +               sdp_record_free(rec);
> +       }
>  }
>
>  static void services_changed(struct btd_device *device)
>

My mail to linux bluetooth bounced yesterday. Sending it again.

The req->profiles_removed gets updated from device->uuids.
However, in the case, where we are paired with the remote device and
we unpair and then repair, since we will free device->uuids on the
unpair, the req->profiles_removed won't get updated after the SDP
query on repair. And so we would need to update the storage from
device->uuids.

Since req->profiles_removed would be empty, device_remove_drivers
won't get called. Also I believe it makes sense to have 2 functions,
even though there is slight overhead, since the deletion of SDP
records from storage, and removing device_probe_drivers are 2 distinct
operations.

Coding style, cast issue, memory leak fixed, patch attached.

>
> --
> Luiz Augusto von Dentz
> Engenheiro de Computação

[-- Attachment #2: 0002-Update-SDP-storage-records-when-a-record-is-deleted.patch --]
[-- Type: application/octet-stream, Size: 3262 bytes --]

From 45fc714460b82c0b3d82d73d00582d9a5618d267 Mon Sep 17 00:00:00 2001
From: Jaikumar Ganesh <jaikumar@google.com>
Date: Thu, 22 Oct 2009 16:17:17 -0700
Subject: [PATCH] Update SDP storage records when a record is deleted.

When a SDP record is deleted at the remote end, we update
the storage only if we have a device driver registered for that UUID.
Update the cache in all cases.
---
src/device.c | 52 ++++++++++++++++++++++++++++++++--------------------
1 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/device.c b/src/device.c
index 63f35de..2b0c80a 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1110,13 +1110,15 @@ void device_probe_drivers(struct btd_device *device, GSList *profiles)
}
}

-static void device_remove_drivers(struct btd_device *device, GSList *uuids)
+static void update_service_records(struct btd_device *device)
{
struct btd_adapter *adapter = device_get_adapter(device);
- GSList *list, *next;
- char srcaddr[18], dstaddr[18];
bdaddr_t src;
- sdp_list_t *records;
+ sdp_list_t *records, *seq;
+ sdp_record_t *rec;
+ char srcaddr[18], dstaddr[18];
+ GSList *l;
+ const char *uuid;

adapter_get_address(adapter, &src);
ba2str(&src, srcaddr);
@@ -1124,6 +1126,28 @@ static void device_remove_drivers(struct btd_device *device, GSList *uuids)

records = read_records(&src, &device->bdaddr);

+ for (l = device->uuids; l; l = l->next) {
+ uuid = l->data;
+ rec = find_record_in_list(records, uuid);
+ if (rec) {
+ records = sdp_list_remove(records, rec);
+ sdp_record_free(rec);
+ }
+ }
+
+ for (seq = records; seq; seq = seq->next) {
+ rec = seq->data;
+ delete_record(srcaddr, dstaddr, rec->handle);
+ }
+
+ if (records)
+ sdp_list_free(records, (sdp_free_func_t) sdp_record_free);
+}
+
+static void device_remove_drivers(struct btd_device *device, GSList *uuids)
+{
+ GSList *list, *next;
+
debug("Remove drivers for %s", device->path);

for (list = device->drivers; list; list = next) {
@@ -1134,36 +1158,22 @@ static void device_remove_drivers(struct btd_device *device, GSList *uuids)
next = list->next;

for (uuid = driver->uuids; *uuid; uuid++) {
- sdp_record_t *rec;
-
if (!g_slist_find_custom(uuids, *uuid,
(GCompareFunc) strcasecmp))
continue;

- debug("UUID %s was removed from device %s",
- *uuid, dstaddr);
+ debug("UUID %s was removed from device path %s",
+ *uuid, device->path);

driver->remove(device);
device->drivers = g_slist_remove(device->drivers,
driver_data);
g_free(driver_data);

- rec = find_record_in_list(records, *uuid);
- if (!rec)
- break;
-
- delete_record(srcaddr, dstaddr, rec->handle);
-
- records = sdp_list_remove(records, rec);
- sdp_record_free(rec);
-
break;
}
}

- if (records)
- sdp_list_free(records, (sdp_free_func_t) sdp_record_free);
-
for (list = uuids; list; list = list->next)
device->uuids = g_slist_remove(device->uuids, list->data);
}
@@ -1333,6 +1343,8 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
if (req->profiles_removed)
device_remove_drivers(device, req->profiles_removed);

+ update_service_records(device);
+
/* Propagate services changes */
services_changed(req->device);

--
1.6.2.3