Re: [Bluez-devel] [PATCH] Make RemoteName D-BUS method non-blocking

[Johan Hedberg <johan.hedberg@nokia.com>]

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

On Thu, Oct 20, 2005, Marcel Holtmann wrote:
> > > Why don't you set rq.event to EVT_CMD_STATUS?
> > 
> > Because hci_send_req has a switch statement which handles the
> > EVT_CMD_STATUS case separately and would never reach the case where it
> > tests for hdr->evt == r->event.
> 
> This looks like a bug to me. We should check r->event == EVT_CMD_STATUS
> and then exit nicely.

Yeah, that sounds like the best solution to me. Attached is a patch to
fix it, as well as a new patch for hcid which sets rq.event to
EVT_CMD_STATUS.

> > Attached is a new patch which should apply cleanly to CVS.
> 
> Do you think the status parameter is needed? Is it not better to send
> two different responses? One for the success case and one for the
> failure case.

I'm not sure whether the status parameter is needed, perhaps if someone
wants to make an UI which shows an accurate error message why the name
request failed? Anyway, I don't have a strong opinion about it, so if
you want to I can remove the status parameter and split the signal to
RemoteName and RemoteNameFailed signals.

Johan

[-- Attachment #2: send-req.patch --]
[-- Type: text/plain, Size: 514 bytes --]

Index: src/hci.c
===================================================================
RCS file: /cvsroot/bluez/libs/src/hci.c,v
retrieving revision 1.81
diff -u -r1.81 hci.c
--- src/hci.c	22 Sep 2005 23:10:48 -0000	1.81
+++ src/hci.c	20 Oct 2005 08:13:12 -0000
@@ -972,6 +972,12 @@
 			if (cs->opcode != opcode)
 				continue;
 
+			if (hdr->evt == r->event) {
+				r->rlen = MIN(len, r->rlen);
+				memcpy(r->rparam, ptr, r->rlen);
+				goto done;
+			}
+
 			if (cs->status) {
 				errno = EIO;
 				goto failed;

[-- Attachment #3: remote-name3.patch --]
[-- Type: text/plain, Size: 5216 bytes --]

Index: hcid/dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.28
diff -u -r1.28 dbus.c
--- hcid/dbus.c	19 Oct 2005 10:52:45 -0000	1.28
+++ hcid/dbus.c	20 Oct 2005 08:32:59 -0000
@@ -466,7 +466,7 @@
 	return;
 }
 
-void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, char *name)
+void hcid_dbus_remote_name(bdaddr_t *local, uint8_t status, bdaddr_t *peer, char *name)
 {
 	DBusMessage *message = NULL;
 	char path[MAX_PATH_LENGTH];
@@ -493,6 +493,7 @@
 	}
 
 	dbus_message_append_args(message,
+					DBUS_TYPE_BYTE, &status,
 					DBUS_TYPE_STRING, &peer_addr,
 					DBUS_TYPE_STRING, &name,
 					DBUS_TYPE_INVALID);
@@ -1207,6 +1208,7 @@
 	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	inquiry_cp cp;
+	evt_cmd_status rp;
 	struct hci_request rq;
 	struct hci_dbus_data *dbus_data = data;
 	int dev_id = -1, dd = -1;
@@ -1250,6 +1252,9 @@
 	rq.ocf = OCF_INQUIRY;
 	rq.cparam = &cp;
 	rq.clen = INQUIRY_CP_SIZE;
+	rq.rparam = &rp;
+	rq.rlen = EVT_CMD_STATUS_SIZE;
+	rq.event = EVT_CMD_STATUS;
 
 	if (hci_send_req(dd, &rq, 100) < 0) {
 		syslog(LOG_ERR, "Unable to start inquiry: %s", strerror(errno));
@@ -1257,6 +1262,12 @@
 		goto failed;
 	}
 
+	if (rp.status) {
+		syslog(LOG_ERR, "Inquiry command failed with status 0x%02X", rp.status);
+		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + EIO);
+		goto failed;
+	}
+
 	reply = dbus_message_new_method_return(msg);
 
 failed:
@@ -1322,8 +1333,6 @@
 
 static DBusMessage* handle_remote_name_req(DBusMessage *msg, void *data)
 {
-	char name[64];
-	const char *pname = name;
 	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	struct hci_dbus_data *dbus_data = data;
@@ -1331,6 +1340,9 @@
 	int dd = -1;
 	const char *str_bdaddr;
 	bdaddr_t bdaddr;
+	struct hci_request rq;
+	remote_name_req_cp cp;
+	evt_cmd_status rp;
 
 	dbus_message_iter_init(msg, &iter);
 	dbus_message_iter_get_basic(&iter, &str_bdaddr);
@@ -1343,27 +1355,47 @@
 			reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 			goto failed;
 		}
-	} else {
+	} else
 		dev_id = dbus_data->id;
-	}
 
 	dd = hci_open_dev(dev_id);
-	if (dd >= 0) {
-		if (hci_read_remote_name(dd, &bdaddr, sizeof(name), name, READ_REMOTE_NAME_TIMEOUT) ==0) {
-			reply = dbus_message_new_method_return(msg);
-			dbus_message_iter_init_append(reply, &iter);
-			dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &pname);
-		} else {
-			reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
-		}
-	} else {
+	if (dd < 0) {
+		syslog(LOG_ERR, "Unable to open device %d: %s", dev_id, strerror(errno));
+		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
+		goto failed;
+	}
+
+	memset(&cp, 0, sizeof(cp));
+	cp.bdaddr = bdaddr;
+	cp.pscan_rep_mode = 0x01;
+
+	memset(&rq, 0, sizeof(rq));
+	rq.ogf = OGF_LINK_CTL;
+	rq.ocf = OCF_REMOTE_NAME_REQ;
+	rq.cparam = &cp;
+	rq.clen = REMOTE_NAME_REQ_CP_SIZE;
+	rq.rparam = &rp;
+	rq.rlen = EVT_CMD_STATUS_SIZE;
+	rq.event = EVT_CMD_STATUS;
+
+	if (hci_send_req(dd, &rq, 100) < 0) {
+		syslog(LOG_ERR, "Unable to send remote name request: %s", strerror(errno));
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
+		goto failed;
 	}
 
-	if (dd > 0)
-		close(dd);
+	if (rp.status) {
+		syslog(LOG_ERR, "Remote name command failed with status 0x%02X", rp.status);
+		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + EIO);
+		goto failed;
+	}
+
+	reply = dbus_message_new_method_return(msg);
 
 failed:
+	if (dd >= 0)
+		hci_close_dev(dd);
+
 	return reply;
 }
 
Index: hcid/hcid.h
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/hcid.h,v
retrieving revision 1.25
diff -u -r1.25 hcid.h
--- hcid/hcid.h	9 Oct 2005 22:15:22 -0000	1.25
+++ hcid/hcid.h	20 Oct 2005 08:33:00 -0000
@@ -131,7 +131,7 @@
 void hcid_dbus_inquiry_start(bdaddr_t *local);
 void hcid_dbus_inquiry_complete(bdaddr_t *local);
 void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class, int8_t rssi);
-void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, char *name);
+void hcid_dbus_remote_name(bdaddr_t *local, uint8_t status, bdaddr_t *peer, char *name);
 void hcid_dbus_conn_complete(bdaddr_t *local, bdaddr_t *peer);
 void hcid_dbus_disconn_complete(bdaddr_t *local, bdaddr_t *peer, uint8_t reason);
 #else
Index: hcid/security.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/security.c,v
retrieving revision 1.40
diff -u -r1.40 security.c
--- hcid/security.c	6 Sep 2005 22:01:30 -0000	1.40
+++ hcid/security.c	20 Oct 2005 08:33:00 -0000
@@ -499,17 +499,16 @@
 	char name[249];
 	bdaddr_t dba;
 
-	if (evt->status)
-		return;
+	bacpy(&dba, &evt->bdaddr);
 
 	memset(name, 0, sizeof(name));
-	memcpy(name, evt->name, 248);
 
-	bacpy(&dba, &evt->bdaddr);
-
-	hcid_dbus_remote_name(sba, &dba, name);
+	if (evt->status == 0) {
+		memcpy(name, evt->name, 248);
+		write_device_name(sba, &dba, name);
+	}
 
-	write_device_name(sba, &dba, name);
+	hcid_dbus_remote_name(sba, evt->status, &dba, name);
 }
 
 static inline void remote_version_information(int dev, bdaddr_t *sba, void *ptr)