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 | 48 +++++++++++++++++++++++++++---------------------
1 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/src/device.c b/src/device.c
index 63f35de..9386681 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1110,19 +1110,37 @@ 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);
ba2str(&device->bdaddr, dstaddr);
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);
+ }
+ for (seq = records; seq; seq = seq->next) {
+ rec = (sdp_record_t *) 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);
@@ -1134,36 +1152,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 +1337,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
Hi Jaikumar,
On Thu, Oct 29, 2009, Jaikumar Ganesh wrote:
> On Thu, Oct 29, 2009 at 1:34 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
> > Hi,
> >
> > They look so much better now :D
>
> Attached.
Thanks. Both patches have now been pushed upstream.
Johan
Hi Johan:
On Thu, Oct 29, 2009 at 1:34 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> They look so much better now :D
Attached.
>
> --
> Luiz Augusto von Dentz
> Engenheiro de Computa??o
>
Hi,
They look so much better now :D
--
Luiz Augusto von Dentz
Engenheiro de Computa??o
Hi Johan:
On Thu, Oct 29, 2009 at 11:42 AM, Jaikumar Ganesh <[email protected]> wrote:
> Hi Johan:
>
> On Wed, Oct 28, 2009 at 5:15 PM, Johan Hedberg <[email protected]> wrote:
>> Hi,
>>
>> On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
>>> > On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
>>> >> The SDP cache is ?removed when the device is removed. In fact, you
>>> >> fixed this a few days back.
>>> >
>>> > Which commit are you referring to?
>>>
>>> Commit number:
>>>
>>> 4bec43039626e853489e72149014868f8c8afedc
>>>
>>> http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=4bec43039626e853489e72149014868f8c8afedc
>>
>> Ok, so we're talking about different things here. I was talking about
>> removing the records from /var/lib/bluetooth/...
>>
>>> We can also fix this by removing the on disk SDP records when the
>>> device is freed.
>>> (I think which is what you suggested - by cache I was interpreting it
>>> as in memory cache)
>>> Will submit a new patch. Let me know if I read you wrong.
>>
>> No, you're right. I was referring to the disk cache. I'm glad we have this
>> misunderstanding finally sorted out :)
>
> ? New patch attached.
The patch broken into 2 patches are attached.
>
>>
>> Johan
>>
>
Hi Johan:
On Wed, Oct 28, 2009 at 5:15 PM, Johan Hedberg <[email protected]> wrote:
> Hi,
>
> On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
>> > On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
>> >> The SDP cache is ?removed when the device is removed. In fact, you
>> >> fixed this a few days back.
>> >
>> > Which commit are you referring to?
>>
>> Commit number:
>>
>> 4bec43039626e853489e72149014868f8c8afedc
>>
>> http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=4bec43039626e853489e72149014868f8c8afedc
>
> Ok, so we're talking about different things here. I was talking about
> removing the records from /var/lib/bluetooth/...
>
>> We can also fix this by removing the on disk SDP records when the
>> device is freed.
>> (I think which is what you suggested - by cache I was interpreting it
>> as in memory cache)
>> Will submit a new patch. Let me know if I read you wrong.
>
> No, you're right. I was referring to the disk cache. I'm glad we have this
> misunderstanding finally sorted out :)
New patch attached.
>
> Johan
>
Hi,
On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
> > On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
> >> The SDP cache is ?removed when the device is removed. In fact, you
> >> fixed this a few days back.
> >
> > Which commit are you referring to?
>
> Commit number:
>
> 4bec43039626e853489e72149014868f8c8afedc
>
> http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=4bec43039626e853489e72149014868f8c8afedc
Ok, so we're talking about different things here. I was talking about
removing the records from /var/lib/bluetooth/...
> We can also fix this by removing the on disk SDP records when the
> device is freed.
> (I think which is what you suggested - by cache I was interpreting it
> as in memory cache)
> Will submit a new patch. Let me know if I read you wrong.
No, you're right. I was referring to the disk cache. I'm glad we have this
misunderstanding finally sorted out :)
Johan
Hi Johan:
On Wed, Oct 28, 2009 at 4:54 PM, Johan Hedberg <[email protected]> wrote:
> Hi,
>
> On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
>> The SDP cache is ?removed when the device is removed. In fact, you
>> fixed this a few days back.
>
> Which commit are you referring to?
Commit number:
4bec43039626e853489e72149014868f8c8afedc
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=4bec43039626e853489e72149014868f8c8afedc
>
>> This is the scenario I am referring to:
>> a) Device is paired and SDP records fetched.
>> b) Device is unpaired - device cache records of SDP is freed
>
> Ok, so at this point we don't have anything stored about the device,
> neither on disk nor in runtime memory.
I should have been more clear - we only remove the runtime memory cache
The on disk SDP records are not removed.
>
>> c) On Remote Device - some SDP records are deleted.
>> d) When the device is created again and the SDP records fetched,
>> profiles_removed is empty and so
>>
>> ? ? ? ? if (req->profiles_removed)
>> ? ? ? ? ? ? ? ? device_remove_drivers(device, req->profiles_removed);
>>
>> device_remove_drivers will not be called and hence SDP records will
>> never get deleted.
>
> But what is there to delete if in step b) we already deleted everything?
We can also fix this by removing the on disk SDP records when the
device is freed.
(I think which is what you suggested - by cache I was interpreting it
as in memory cache)
Will submit a new patch. Let me know if I read you wrong.
>
> Johan
>
Hi,
On Wed, Oct 28, 2009, Jaikumar Ganesh wrote:
> The SDP cache is removed when the device is removed. In fact, you
> fixed this a few days back.
Which commit are you referring to?
> This is the scenario I am referring to:
> a) Device is paired and SDP records fetched.
> b) Device is unpaired - device cache records of SDP is freed
Ok, so at this point we don't have anything stored about the device,
neither on disk nor in runtime memory.
> c) On Remote Device - some SDP records are deleted.
> d) When the device is created again and the SDP records fetched,
> profiles_removed is empty and so
>
> if (req->profiles_removed)
> device_remove_drivers(device, req->profiles_removed);
>
> device_remove_drivers will not be called and hence SDP records will
> never get deleted.
But what is there to delete if in step b) we already deleted everything?
Johan
Hi Johan:
On Wed, Oct 28, 2009 at 4:34 PM, Johan Hedberg <[email protected]> wrote:
> Hi Jaikumar,
>
> On Mon, Oct 26, 2009, Jaikumar Ganesh wrote:
>> When the device is removed, we remove the cache and remove the device->uuids
>> value. So when the device is created the next time, we cannot depend on
>> the profiles_removed value, we need to reconstruct from device->uuids.
>> device_remove_drivers call is based on the profiles_removed flag and hence that
>> call will never be made, when a new device is created.
>>
>> My change removes the dependency of SDP records and the probe_drivers,
>> Thus, removal of SDP records doesn't depend on the profiles_removed flag
>> but rather uses device->uuids directly.
>>
>> profiles_added and profiles_removed flags are useful in the case where
>> the user calls discover services on an already created device and there
>> are changes in the SDP records.
>
> If a device was removed then we shouldn't have any records stored records
> for it left in the cache and so it shouldn't matter that the
> profiles_removed list is empty when the device is recreated again, right?
> It seems there's a bug in the device_remove_stored function in device.c
> since it doesn't remove the record cache, so that's at least something
> that should be fixed.
>
The SDP cache is removed when the device is removed. In fact, you
fixed this a few days back.
This is the scenario I am referring to:
a) Device is paired and SDP records fetched.
b) Device is unpaired - device cache records of SDP is freed
c) On Remote Device - some SDP records are deleted.
d) When the device is created again and the SDP records fetched,
profiles_removed is empty and so
if (req->profiles_removed)
device_remove_drivers(device, req->profiles_removed);
device_remove_drivers will not be called and hence SDP records will
never get deleted.
e) My change removes the coupling of SDP records with the drivers code
and hence we will remove the SDP records in this scenario.
> Johan
>
Thanks
Jaikumar
Hi Jaikumar,
On Mon, Oct 26, 2009, Jaikumar Ganesh wrote:
> When the device is removed, we remove the cache and remove the device->uuids
> value. So when the device is created the next time, we cannot depend on
> the profiles_removed value, we need to reconstruct from device->uuids.
> device_remove_drivers call is based on the profiles_removed flag and hence that
> call will never be made, when a new device is created.
>
> My change removes the dependency of SDP records and the probe_drivers,
> Thus, removal of SDP records doesn't depend on the profiles_removed flag
> but rather uses device->uuids directly.
>
> profiles_added and profiles_removed flags are useful in the case where
> the user calls discover services on an already created device and there
> are changes in the SDP records.
If a device was removed then we shouldn't have any records stored records
for it left in the cache and so it shouldn't matter that the
profiles_removed list is empty when the device is recreated again, right?
It seems there's a bug in the device_remove_stored function in device.c
since it doesn't remove the record cache, so that's at least something
that should be fixed.
Johan
Hey Luiz, Johan:
On Mon, Oct 26, 2009 at 8:20 AM, Jaikumar Ganesh <[email protected]> wrote:
> Hi Luiz:
>
> On Fri, Oct 23, 2009 at 10:44 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>>
>> Hi,
>>
>> On Fri, Oct 23, 2009 at 12:17 PM, Jaikumar Ganesh <[email protected]> wrote:
>> >
>> > 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.
>>
>> Im not sure if I get you right, but if you are saying that when device
>> object is gone profiles_removed is empty that is the a new device and
>> we shouldn't have any profile at that point for consistence, so either
>> we remove the sdp cache on device_remove_stored or we never remove
>> records from the cache. Perhaps the record cache is really useless and
>> we should only cache the uuids information and query the record on
>> every connect attempt (we are going to connect anyway so the cost is
>> about the same.)
>
> When the device is removed, we remove the cache and remove the device->uuids
> value. So when the device is created the next time, we cannot depend on
> the profiles_removed value, we need to reconstruct from device->uuids.
> device_remove_drivers call is based on the profiles_removed flag and hence that
> call will never be made, when a new device is created.
>
> My change removes the dependency of SDP records and the probe_drivers,
> Thus, removal of SDP records doesn't depend on the profiles_removed flag
> but rather uses device->uuids directly.
>
> profiles_added and profiles_removed flags are useful in the case where the user
> calls discover services on an already created device and there are
> changes in the SDP
> records.
>
>>
>> > 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.
>>
>> Maybe if you called it on device_remove_stored too, but if we are
>> going to validate that information on every connection attempt I doubt
>> this is useful in the end.
>>
>> --
>> Luiz Augusto von Dentz
>> Engenheiro de Computa??o
>
> Thanks
> Jaikumar
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
Any update on this ?
Thanks
Jaikumar
Hi Luiz:
On Fri, Oct 23, 2009 at 10:44 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi,
>
> On Fri, Oct 23, 2009 at 12:17 PM, Jaikumar Ganesh <[email protected]> wrote:
> >
> > 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.
>
> Im not sure if I get you right, but if you are saying that when device
> object is gone profiles_removed is empty that is the a new device and
> we shouldn't have any profile at that point for consistence, so either
> we remove the sdp cache on device_remove_stored or we never remove
> records from the cache. Perhaps the record cache is really useless and
> we should only cache the uuids information and query the record on
> every connect attempt (we are going to connect anyway so the cost is
> about the same.)
When the device is removed, we remove the cache and remove the device->uuids
value. So when the device is created the next time, we cannot depend on
the profiles_removed value, we need to reconstruct from device->uuids.
device_remove_drivers call is based on the profiles_removed flag and hence that
call will never be made, when a new device is created.
My change removes the dependency of SDP records and the probe_drivers,
Thus, removal of SDP records doesn't depend on the profiles_removed flag
but rather uses device->uuids directly.
profiles_added and profiles_removed flags are useful in the case where the user
calls discover services on an already created device and there are
changes in the SDP
records.
>
> > 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.
>
> Maybe if you called it on device_remove_stored too, but if we are
> going to validate that information on every connection attempt I doubt
> this is useful in the end.
>
> --
> Luiz Augusto von Dentz
> Engenheiro de Computa??o
Thanks
Jaikumar
Hi,
On Fri, Oct 23, 2009 at 12:17 PM, Jaikumar Ganesh <[email protected]> wrote:
>
> 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.
Im not sure if I get you right, but if you are saying that when device
object is gone profiles_removed is empty that is the a new device and
we shouldn't have any profile at that point for consistence, so either
we remove the sdp cache on device_remove_stored or we never remove
records from the cache. Perhaps the record cache is really useless and
we should only cache the uuids information and query the record on
every connect attempt (we are going to connect anyway so the cost is
about the same.)
> 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.
Maybe if you called it on device_remove_stored too, but if we are
going to validate that information on every connection attempt I doubt
this is useful in the end.
--
Luiz Augusto von Dentz
Engenheiro de Computa??o
Hi Luiz:
On Fri, Oct 23, 2009 at 6:49 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi,
>
> On Thu, Oct 22, 2009 at 8:19 PM, Jaikumar Ganesh <[email protected]> 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
Hi,
On Thu, Oct 22, 2009 at 8:19 PM, Jaikumar Ganesh <[email protected]> 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)
--
Luiz Augusto von Dentz
Engenheiro de Computa??o
Hi Jaikumar,
A few quick coments on this:
On Thu, Oct 22, 2009, Jaikumar Ganesh wrote:
> +static void update_service_records(struct btd_device *device) {
Coding style: opening bracket of a function on its own line.
> 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);
> + }
It looks like you're leaking memory here. I guess rec should be freed
after the sdp_list_remove.
> + for (seq = records; seq; seq = seq->next) {
> + rec = (sdp_record_t *) seq->data;
Since seq->data is void* the cast is unnecessary here.
> + delete_record(srcaddr, dstaddr, rec->handle);
> + }
> + if (records)
> + sdp_list_free(records, (sdp_free_func_t) sdp_record_free);
> +}
Coding style: I'd add empty lines between the for-loops as well as before
the if-statement here.
Other than those issues one thing that bothers me a little is that it
seems the req->profiles_removed list isn't used anymore at this point but
a list of removed records is built from scratch a second time. Are you
sure that this existing list of removed UUIDs couldn't be used somehow to
make the code more efficient? It might well be that this I'm missing
something and this list can't be used but I thought it'd be good to check
that you've considered this possibility nevertheless.
Johan