bug and possible patch for dbus DiscoverServices

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

 



Hi Bluetooth people

I am investigating a bug where dbus DiscoverServices incorrectly returns
an empty array in some circumstances (see below).

Here is my suggested patch, against BlueZ 4.47 (I don't think there are
any more recent changes in this area):

====
diff -ur ../eclair.old/external/bluetooth/bluez/src/device.c external/bluetooth/bluez/src/device.c
--- ../eclair.old/external/bluetooth/bluez/src/device.c	2009-12-15 13:15:44.000000000 +0000
+++ external/bluetooth/bluez/src/device.c	2010-06-10 11:31:04.000000000 +0100
@@ -1380,6 +1380,7 @@
 {
 	struct browse_req *req = user_data;
 	struct btd_device *device = req->device;
+	sdp_list_t	*records = 0;
 	DBusMessage *reply;
 
 	if (err < 0) {
@@ -1389,6 +1390,7 @@
 	}
 
 	update_services(req, recs);
+	records = req->records;
 
 	if (device->tmp_records && req->records) {
 		sdp_list_free(device->tmp_records,
@@ -1422,7 +1424,7 @@
 
 	if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
 					"DiscoverServices")) {
-		discover_services_reply(req, err, req->records);
+		discover_services_reply(req, err, records);
 		goto cleanup;
 	}
 
====

The problem seems to arise in this bit of search_cb() at device.c:1395:

    if (device->tmp_records && req->records) {
        sdp_list_free(device->tmp_records,
                    (sdp_free_func_t) sdp_record_free);
        device->tmp_records = req->records;
        req->records = NULL;
    }

In the case that device->tmp_records was already set, this zeroes
req->records and so causes the empty array to be returned via dbus a bit
lower down in the function.

I'm not really sure what tmp_records does, and whether it should be
non-zero at this point. So it could be either

1. tmp_records could be non-zero, so search_cb should return the results
even if the above code kicks in and zeroes req->records (that's what my
patch assumes); or

2. tmp_records is supposed to be zero there, so there is a bug
elsewhere. In particular, I have only seen the bug on Android phones,
not on my PC, and a bit of investigation showed that it is some Android
specific code (the new dbus method GetServiceAttributeValue; see the
device.c here:
http://android.git.kernel.org/?p=platform/external/bluez.git;a=blob;f=src/device.c;h=67a77318ad136c12cad96f0e537f8ceac8ae62f0;hb=refs/heads/eclair-bluez
line 789) that sets tmp_records and does not clear it again.

Also, the bug does not appear when the device I am trying to discover
services on is my PC rather than a phone. I suspect the Android
Bluetooth startup sequence only uses this GetServiceAttributeValue on a
remote device with some phone-specific service, so the
device->tmp_records for my PC remains at 0.

I would be grateful for any hints on what tmp_records does, and whether
my patch is correct or whether I should be looking elsewhere.

Thanks.

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