[Bluez-devel] [PATCH] DBus GetRemoteName problem

[Bluez-devel] [PATCH] DBus GetRemoteName problem

[Frederic Danis]

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

Hello,

When I use GetRemoteName of dbus-test program I receive a 
DiscoveryCompleted signal after the RemoteNameUpdated (or 
RemoteNameFailed) signal.
You can find attached a patch to fix this.

Hope this helps.

Regards

Fred

-- 
-----------------------------------------------
It is not by improving the oil lamp that one invents the electric bulb!
-----------------------------------------------
Danis Frederic            PalmSource Europe
Software engineer
Mail : mailto:frederic.danis@palmsource.com
-----------------------------------------------


[-- Attachment #2: fix_dbus_get_remote_name.patch --]
[-- Type: text/plain, Size: 430 bytes --]

--- dbus-adapter.c.orig	2006-06-06 11:35:32.000000000 +0200
+++ dbus-adapter.c	2006-06-06 14:51:57.000000000 +0200
@@ -1255,8 +1255,7 @@
 	 * Otherwise, send the HCI cmd to get the remote name
 	 */
 	if (dbus_data->discover_state == STATE_IDLE) {
-		if (!disc_device_req_name(dbus_data))
-			dbus_data->discover_state = STATE_RESOLVING_NAMES;
+		disc_device_req_name(dbus_data);
 	}
 
 	return error_request_deferred(conn, msg);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] DBus GetRemoteName problem

[Claudio Takahasi]

Hi Frederic,

It's necessary consider some extra conditions:

1. DiscoverDevices running
2. DiscoverDevicesWithoutNameResolving running
3. non D-Bus inquiry request(eg: hcitool, kbluetoothd, ..)
4. After receive a connection complete event, a HCI remote  name
request is sent to update the remote name

The GetRemoteName request can arrive under these conditions.

I already fixed this bug, but it needs more tests.

Claudio.

-- 
---------------------------------------------------------
Claudio Takahasi
Instituto Nokia de Tecnologia - INdT

On 6/6/06, Frederic Danis <frederic.danis@palmsource.com> wrote:
> Hello,
>
> When I use GetRemoteName of dbus-test program I receive a
> DiscoveryCompleted signal after the RemoteNameUpdated (or
> RemoteNameFailed) signal.
> You can find attached a patch to fix this.
>
> Hope this helps.
>
> Regards
>
> Fred
>
> --
> -----------------------------------------------
> It is not by improving the oil lamp that one invents the electric bulb!
> -----------------------------------------------
> Danis Frederic            PalmSource Europe
> Software engineer
> Mail : mailto:frederic.danis@palmsource.com
> -----------------------------------------------
>
>
>
> --- dbus-adapter.c.orig 2006-06-06 11:35:32.000000000 +0200
> +++ dbus-adapter.c      2006-06-06 14:51:57.000000000 +0200
> @@ -1255,8 +1255,7 @@
>          * Otherwise, send the HCI cmd to get the remote name
>          */
>         if (dbus_data->discover_state == STATE_IDLE) {
> -               if (!disc_device_req_name(dbus_data))
> -                       dbus_data->discover_state = STATE_RESOLVING_NAMES;
> +               disc_device_req_name(dbus_data);
>         }
>
>         return error_request_deferred(conn, msg);
>
>
>
>
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>
>
>


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] DBus GetRemoteName problem

[Claudio Takahasi]

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

Hi Marcel, Fred,

Here is the patch to fix the extra DiscoveryCompleted signal sent. I
added an extra field in the discovered_dev_info list to handle the
scenario when GetRemoteName request is received while there is a
DiscoverDevicesWithoutNameResolving running.

I also included some code cleanup.

Do you have comments or another suggestion to fix this problem?

Claudio.

On 6/6/06, Claudio Takahasi <cktakahasi@gmail.com> wrote:
> Hi Frederic,
>
> It's necessary consider some extra conditions:
>
> 1. DiscoverDevices running
> 2. DiscoverDevicesWithoutNameResolving running
> 3. non D-Bus inquiry request(eg: hcitool, kbluetoothd, ..)
> 4. After receive a connection complete event, a HCI remote  name
> request is sent to update the remote name
>
> The GetRemoteName request can arrive under these conditions.
>
> I already fixed this bug, but it needs more tests.
>
> Claudio.
>
> --
> ---------------------------------------------------------
> Claudio Takahasi
> Instituto Nokia de Tecnologia - INdT
>
> On 6/6/06, Frederic Danis <frederic.danis@palmsource.com> wrote:
> > Hello,
> >
> > When I use GetRemoteName of dbus-test program I receive a
> > DiscoveryCompleted signal after the RemoteNameUpdated (or
> > RemoteNameFailed) signal.
> > You can find attached a patch to fix this.
> >
> > Hope this helps.
> >
> > Regards
> >
> > Fred
> >
> > --
> > -----------------------------------------------
> > It is not by improving the oil lamp that one invents the electric bulb!
> > -----------------------------------------------
> > Danis Frederic            PalmSource Europe
> > Software engineer
> > Mail : mailto:frederic.danis@palmsource.com
> > -----------------------------------------------
> >
> >
> >
> > --- dbus-adapter.c.orig 2006-06-06 11:35:32.000000000 +0200
> > +++ dbus-adapter.c      2006-06-06 14:51:57.000000000 +0200
> > @@ -1255,8 +1255,7 @@
> >          * Otherwise, send the HCI cmd to get the remote name
> >          */
> >         if (dbus_data->discover_state == STATE_IDLE) {
> > -               if (!disc_device_req_name(dbus_data))
> > -                       dbus_data->discover_state = STATE_RESOLVING_NAMES;
> > +               disc_device_req_name(dbus_data);
> >         }
> >
> >         return error_request_deferred(conn, msg);
> >
> >
> >
> >
> > _______________________________________________
> > Bluez-devel mailing list
> > Bluez-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/bluez-devel
> >
> >
> >
>


-- 
---------------------------------------------------------
Claudio Takahasi
Instituto Nokia de Tecnologia - INdT

[-- Attachment #2: discover02.patch --]
[-- Type: text/x-patch, Size: 11682 bytes --]

Index: dbus-adapter.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus-adapter.c,v
retrieving revision 1.55
diff -u -r1.55 dbus-adapter.c
--- dbus-adapter.c	6 Jun 2006 18:02:21 -0000	1.55
+++ dbus-adapter.c	7 Jun 2006 12:30:46 -0000
@@ -1248,16 +1248,14 @@
 
 	/* put the request name in the queue to resolve name */
 	str2ba(peer_addr, &peer_bdaddr);
-	disc_device_append(&dbus_data->disc_devices, &peer_bdaddr, NAME_PENDING);
+	disc_device_append(&dbus_data->disc_devices, &peer_bdaddr, NAME_PENDING, RESOLVE_NAME);
 
 	/* 
 	 * if there is a discover process running, just queue the request.
 	 * Otherwise, send the HCI cmd to get the remote name
 	 */
-	if (dbus_data->discover_state == STATE_IDLE) {
-		if (!disc_device_req_name(dbus_data))
-			dbus_data->discover_state = STATE_RESOLVING_NAMES;
-	}
+	if (dbus_data->discover_state == STATE_IDLE)
+		disc_device_req_name(dbus_data);
 
 	return error_request_deferred(conn, msg);
 }
@@ -2070,7 +2068,7 @@
 	if (strcmp("DiscoverDevicesWithoutNameResolving", method) == 0)
 		dbus_data->discover_type = WITHOUT_NAME_RESOLVING;
 	else 
-		dbus_data->discover_type = RESOLVE_NAMES;
+		dbus_data->discover_type = RESOLVE_NAME;
 		
 	dbus_data->requestor_name = strdup(dbus_message_get_sender(msg));
 
Index: dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.126
diff -u -r1.126 dbus.c
--- dbus.c	6 Jun 2006 13:28:21 -0000	1.126
+++ dbus.c	7 Jun 2006 12:30:47 -0000
@@ -91,47 +91,58 @@
 		free(dev);
 }
 
-static int disc_device_find_by_bdaddr(const void *data, const void *user_data)
+static int disc_device_find(const struct discovered_dev_info *d1, const struct discovered_dev_info *d2)
 {
-	const struct discovered_dev_info *dev = data;
-	const bdaddr_t *bdaddr = user_data;
+	int ret;
 
-	return bacmp(&dev->bdaddr, bdaddr);
-}
+	if (bacmp(&d2->bdaddr, BDADDR_ANY)) {
+		ret = bacmp(&d1->bdaddr, &d2->bdaddr);
+		if (ret)
+			return ret;
+	}
 
-static int disc_device_find_by_name_status(const void *data, const void *user_data)
-{
-	const struct discovered_dev_info *dev = data;
-	const name_status_t *name_status = user_data;
+	/* if not any */
+	if (d2->name_status) {
+		ret = (d1->name_status - d2->name_status);
+		if (ret)
+			return ret;
+	}
 
-	if (dev->name_status == *name_status)
-		return 0;
+	if (d2->discover_type)
+		return (d1->discover_type - d2->discover_type);
 
-	return -1;
+	return 0;
 }
 
-int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status)
+int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status, int discover_type)
 {
-	struct discovered_dev_info *dev = NULL;
+	struct discovered_dev_info *dev = NULL, match;
 	struct slist *l;
 
-	/* ignore repeated entries */
-	l = slist_find(*list, bdaddr, disc_device_find_by_bdaddr);
+	memset(&match, 0, sizeof(struct discovered_dev_info));
+	bacpy(&match.bdaddr, bdaddr);
+	match.name_status = NAME_ANY;
 
+	/* ignore repeated entries */
+	l = slist_find(*list, &match, (cmp_func_t)disc_device_find);
 	if (l) {
 		/* device found, update the attributes */
 		dev = l->data;
 		dev->name_status = name_status;
-		return -1;
+		/* get remote name received while discovering */
+		if (dev->discover_type != RESOLVE_NAME)
+			dev->discover_type = discover_type; 
+		return -EALREADY;
 	}
 
 	dev = malloc(sizeof(*dev));
 	if (!dev)
-		return -1;
+		return -ENOMEM;
 
 	memset(dev, 0, sizeof(*dev));
 	bacpy(&dev->bdaddr, bdaddr);
 	dev->name_status = name_status;
+	dev->discover_type = discover_type;
 
 	*list = slist_append(*list, dev);
 
@@ -140,11 +151,14 @@
 
 static int disc_device_remove(struct slist **list, bdaddr_t *bdaddr)
 {
-	struct discovered_dev_info *dev;
+	struct discovered_dev_info *dev, match;
 	struct slist *l;
 	int ret_val = -1;
 
-	l = slist_find(*list, bdaddr, disc_device_find_by_bdaddr);
+	memset(&match, 0, sizeof(struct discovered_dev_info));
+	bacpy(&match.bdaddr, bdaddr);
+
+	l = slist_find(*list, &match, (cmp_func_t)disc_device_find);
 
 	if (l) {
 		dev = l->data;
@@ -341,6 +355,7 @@
 	data->dev_id = dev_id;
 	data->mode = SCAN_DISABLED;
 	data->discoverable_timeout = get_discoverable_timeout(dev_id);
+	data->discover_type = WITHOUT_NAME_RESOLVING; /* default discover type */
 
 	if (fallback) {
 		if (!dbus_connection_register_fallback(connection, path, pvtable, data)) {
@@ -751,29 +766,32 @@
 	evt_cmd_status rp;
 	remote_name_req_cp cp;
 	bdaddr_t tmp;
-	struct discovered_dev_info *dev;
+	struct discovered_dev_info *dev, match;
 	DBusMessage *message = NULL;
 	struct slist *l = NULL;
 	char *peer_addr = NULL;
-	int dd, req_sent, ret_val = 0;
-	name_status_t name_status = NAME_PENDING;
+	int dd, req_sent, ret_val = -ENODATA;
 
-	/*get the next remote address */
+	/* get the next remote address */
 	if (!dbus_data->disc_devices)
-		return -1;
+		return ret_val;
 
-	l = slist_find(dbus_data->disc_devices, &name_status, disc_device_find_by_name_status);
+	memset(&match, 0, sizeof(struct discovered_dev_info));
+	bacpy(&match.bdaddr, BDADDR_ANY);
+	match.name_status = NAME_PENDING;
+	match.discover_type = RESOLVE_NAME;
 
+	l = slist_find(dbus_data->disc_devices, &match, (cmp_func_t)disc_device_find);
 	if (!l)
-		return -1;
+		return ret_val;
 
 	dev = l->data;
 	if (!dev)
-		return -1;
+		return ret_val;
 
 	dd = hci_open_dev(dbus_data->dev_id);
 	if (dd < 0)
-		return -1;
+		return -errno;
 
 	memset(&rq, 0, sizeof(rq));
 	rq.ogf    = OGF_LINK_CTL;
@@ -822,18 +840,18 @@
 			free(dev);
 
 			/* get the next element */
-			l = slist_find(dbus_data->disc_devices, &name_status, disc_device_find_by_name_status);
+			l = slist_find(dbus_data->disc_devices, &match, (cmp_func_t)disc_device_find);
 
-			if (!l) {
-				/* no more devices: exit */
-				ret_val = -1;
+			/* no more devices: exit */
+			if (!l)
 				goto failed;
-			}
 
 			dev = l->data;
 		}
 	} while (!req_sent);
 
+	ret_val = 0;
+
 failed:
 	hci_close_dev(dd);
 
@@ -859,44 +877,37 @@
 
 	snprintf(path, sizeof(path), "%s/hci%d", BASE_PATH, id);
 
-	if (dbus_connection_get_object_path_data(connection, path, (void *) &pdata)) {
-
-		if (pdata->discover_type == RESOLVE_NAMES) {
-			if (!disc_device_req_name(pdata)) {
-				pdata->discover_state = STATE_RESOLVING_NAMES;
-				goto failed; /* skip - there is name to resolve */
-			}
-		}
-		pdata->discover_state = STATE_IDLE;
+	if (!dbus_connection_get_object_path_data(connection, path, (void *) &pdata)) {
+		error("Getting %s path data failed!", path);
+		goto failed;
+	}
 
-		/* free discovered devices list */
-		slist_foreach(pdata->disc_devices, disc_device_info_free, NULL);
-		slist_free(pdata->disc_devices);
-		pdata->disc_devices = NULL;
+	/* reset the discover type to be able to handle D-Bus and non D-Bus requests */
+	pdata->discover_type = WITHOUT_NAME_RESOLVING;
 
-		if (pdata->requestor_name) {
-			free(pdata->requestor_name);
-			pdata->requestor_name = NULL;
-		}
+	if (!disc_device_req_name(pdata)) {
+		pdata->discover_state = STATE_RESOLVING_NAMES;
+		goto failed; /* skip - there is name to resolve */
 	}
 
-	message = dbus_message_new_signal(path, ADAPTER_INTERFACE,
-						"DiscoveryCompleted");
-	if (message == NULL) {
-		error("Can't allocate D-Bus message");
-		goto failed;
-	}
+	pdata->discover_state = STATE_IDLE;
 
-	if (dbus_connection_send(connection, message, NULL) == FALSE) {
-		error("Can't send D-Bus inquiry complete message");
-		goto failed;
+	/* free discovered devices list */
+	slist_foreach(pdata->disc_devices, disc_device_info_free, NULL);
+	slist_free(pdata->disc_devices);
+	pdata->disc_devices = NULL;
+
+	if (pdata->requestor_name) {
+		free(pdata->requestor_name);
+		pdata->requestor_name = NULL;
 	}
 
-	dbus_connection_flush(connection);
+	/* Send discovery completed signal if there isn't name to resolve */
+	message = dbus_message_new_signal(path, ADAPTER_INTERFACE,
+						"DiscoveryCompleted");
+	send_reply_and_unref(connection, message);
 
 failed:
-	if (message)
-		dbus_message_unref(message);
 	bt_free(local_addr);
 }
 
@@ -915,7 +926,7 @@
 	char path[MAX_PATH_LENGTH];
 	struct hci_dbus_data *pdata = NULL;
 	struct slist *l = NULL;
-	struct discovered_dev_info *dev;
+	struct discovered_dev_info match;
 	char *local_addr, *peer_addr, *name = NULL;
 	const char *major_ptr, *minor_ptr;
 	struct slist *service_classes;
@@ -971,23 +982,18 @@
 
 	slist_free(service_classes);
 
-	if (dbus_connection_send(connection, signal_device, NULL) == FALSE) {
-		error("Can't send D-Bus remote device found signal");
-		goto failed;
-	}
+	send_reply_and_unref(connection, signal_device);
 
-	/* send the remote name signal */
-	l = slist_find(pdata->disc_devices, peer, disc_device_find_by_bdaddr);
-
-	if (l) {
-		dev = l->data;
-		if (dev->name_status == NAME_SENT)
-			goto failed; /* don't sent the name again */
-	}
+	memset(&match, 0, sizeof(struct discovered_dev_info));
+	bacpy(&match.bdaddr, peer);
+	match.name_status = NAME_SENT;
+	/* if found: don't sent the name again */
+	l = slist_find(pdata->disc_devices, &match, (cmp_func_t)disc_device_find);
+	if (l)
+		goto failed;
 
 	snprintf(filename, PATH_MAX, "%s/%s/names", STORAGEDIR, local_addr);
 	name = textfile_get(filename, peer_addr);
-
 	if (name) {
 		signal_name = dev_signal_factory(pdata->dev_id, "RemoteNameUpdated",
 							DBUS_TYPE_STRING, &peer_addr,
@@ -998,17 +1004,11 @@
 		free(name);
 		name_status = NAME_SENT;
 	} 
-	/* queue only results triggered by D-Bus clients */
-	if (pdata->requestor_name)
-		disc_device_append(&pdata->disc_devices, peer, name_status);
 
-
-	disc_device_append(&pdata->disc_devices, peer, name_status);
+	/* add in the list to track name sent/pending */
+	disc_device_append(&pdata->disc_devices, peer, name_status, pdata->discover_type);
 
 failed:
-	if (signal_device)
-		dbus_message_unref(signal_device);
-
 	bt_free(local_addr);
 	bt_free(peer_addr);
 }
@@ -1059,7 +1059,11 @@
 	slist_free(pdata->disc_devices);
 	pdata->disc_devices = NULL;
 
-	if (pdata->discover_state == STATE_RESOLVING_NAMES ) {
+	/*
+	 * The discovery completed signal must be sent only for discover 
+	 * devices request WITH name resolving
+	 */
+	if (pdata->discover_state == STATE_RESOLVING_NAMES) {
 		message = dbus_message_new_signal(path, ADAPTER_INTERFACE,
 						  "DiscoveryCompleted");
 
Index: dbus.h
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.h,v
retrieving revision 1.88
diff -u -r1.88 dbus.h
--- dbus.h	6 Jun 2006 18:02:21 -0000	1.88
+++ dbus.h	7 Jun 2006 12:30:47 -0000
@@ -63,13 +63,14 @@
 	STATE_IDLE,
 	STATE_DISCOVER,
 	STATE_RESOLVING_NAMES
-}discover_state_t;
+} discover_state_t;
 
 /* discover type  */
-#define WITHOUT_NAME_RESOLVING		0
-#define RESOLVE_NAMES			1
+#define WITHOUT_NAME_RESOLVING		1 /* D-Bus and non D-Bus request */
+#define RESOLVE_NAME			2	
 
 typedef enum {
+	NAME_ANY,
 	NAME_PENDING,
 	NAME_SENT
 } name_status_t;
@@ -77,6 +78,7 @@
 struct discovered_dev_info {
 	bdaddr_t bdaddr;
 	name_status_t name_status;
+	int discover_type;
 };
 
 struct bonding_request_info {
@@ -190,7 +192,7 @@
 int active_conn_find_by_bdaddr(const void *data, const void *user_data);
 void bonding_request_free(struct bonding_request_info *dev);
 void disc_device_info_free(void *data, void *user_data);
-int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status);
+int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status, int discover_type);
 int disc_device_req_name(struct hci_dbus_data *dbus_data);
 
 int discoverable_timeout_handler(void *data);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel