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

[Jaikumar Ganesh <jaikumar@google.com>]

[-- 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