libhci update

[Iain Hibbert]

Hi Max,
	some more queries I'm not sure about..

--- hci.c.orig	2009-05-14 21:11:04.000000000 +0100
+++ hci.c	2009-05-14 21:26:57.000000000 +0100
@@ -118,7 +118,7 @@ bt_devsend(int s, uint16_t opcode, void
 		h.length = 0;

 	while (writev(s, iv, ivn) < 0) {
-		if (errno == EAGAIN || errno == EINTR)
+		if (errno == EINTR)
 			continue;

 		return (-1);
@@ -163,12 +163,15 @@ bt_devrecv(int s, void *buf, size_t size
 	}

 	while ((n = read(s, buf, size)) < 0) {
-		if (errno == EAGAIN || errno == EINTR)
+		if (errno == EINTR)
 			continue;

 		return (-1);

I think these two should not have EAGAIN because that can be returned if
the socket is set to nonblocking and we should just pass it back to the
caller rather than looping?

 	}

+	if (n == 0)
+		return (0);
+

the read() in bt_devrecv() can return 0 for EOF at least. ok it shouldn't
happen but do you think it would be proper to return it? (or call it an
error? I dunno)

@@ -273,10 +276,11 @@ bt_devreq(int s, struct bt_devreq *r, ti

 				if (r->rlen >= n) {
 					r->rlen = n;
 					memcpy(r->rparam, cc + 1, r->rlen);
 				}
+				/* else what? */

 				goto out;
 			}
 			break;

@@ -290,10 +294,11 @@ bt_devreq(int s, struct bt_devreq *r, ti
 				} else {
 					if (r->rlen >= n) {
 						r->rlen = n;
 						memcpy(r->rparam, cs, r->rlen);
 					}
+					/* else what? */

 					goto out;
 				}
 			}
 			break;
@@ -302,10 +307,11 @@ bt_devreq(int s, struct bt_devreq *r, ti
 			if (e->event == r->event) {
 				if (r->rlen >= n) {
 					r->rlen = n;
 					memcpy(r->rparam, e + 1, r->rlen);
 				}
+				/* else what? */

these three locations in bt_devreq() I'm not sure what should happen if
the provided buffer was not big enough. I suggest the following construct
instead:

	if (r->rlen > n)
		r->rlen = n;

	if (r->rlen > 0)
		memcpy(r->rparam, ..., r->rlen);

which gives them as much as they asked for and discards the rest, how
would that look?

Also, I'm thinking of the case where r->event=0 but a command_status is
returned. If status != 0, an error is produced but if status == 0 then we
should set rlen = 0 and return success?

@@ -504,10 +510,16 @@ wait_for_more:

 	case NG_HCI_EVENT_INQUIRY_RESULT:
 		ir = (ng_hci_inquiry_response *)(ep + 1);

 		for (n = 0; n < MIN(ep->num_responses, num_rsp); n ++) {

I found while writing the inquiry routine in btconfig(8) that some devices
don't filter repeat addresses properly (or maybe its that some devices
continue to respond, I've seen that too). Perhaps its worth handling that
case inside the bt_devinquiry() function by discarding dupes?

@@ -551,11 +563,11 @@ bt_devinfo(struct bt_devinfo *di)
 		return (-1);
 	}

 	s = bt_devopen(di->devname);
 	if (s < 0)
		return (-1);

I'm not sure if in FreeBSD you can connect() to a device that is not
marked up, but in NetBSD you can't. In any case, there is information in
the devinfo structure that is only available from activated devices so it
may fail?

  This really relates to bt_devenum() as below, are we counting 'active'
devices or 'all' devices, as you ignored errors?

@@ -659,21 +671,24 @@ bt_devenum(bt_devenum_cb_t cb, void *arg
 	}

 	for (count = 0, i = 0; i < rp.num_names; i ++) {
 		strlcpy(di.devname, rp.names[i].name, sizeof(di.devname));
 		if (bt_devinfo(&di) < 0)
-			continue;
+			continue;	/* or maybe error? */

 		count ++;

 		if (cb == NULL)
 			continue;

 		strlcpy(ha.hci_node, rp.names[i].name, sizeof(ha.hci_node));
 		if (bind(s, (struct sockaddr *) &ha, sizeof(ha)) < 0 ||
 		    connect(s, (struct sockaddr *) &ha, sizeof(ha)) < 0)
-			continue;
+			continue;	/* or maybe error? */
+

I think the second one should cause an error return at least? Not sure
about the first - in NetBSD I have the flags already so I've skipped
inactive interfaces as below

	while (ioctl(s, SIOCNBTINFO, &btr) != -1) {
		if ((btr.btr_flags & BTF_UP) == 0)
			continue;

		count++;

		if (cb == NULL)
			continue;

		memset(&di, 0, sizeof(di));
		strlcpy(di.devname, btr.btr_name, HCI_DEVNAME_SIZE);

		memset(&sa, 0, sizeof(sa));
		sa.bt_len = sizeof(sa);
		sa.bt_family = AF_BLUETOOTH;
		bdaddr_copy(&sa.bt_bdaddr, &btr.btr_bdaddr);

		if (bt_devinfo(&di) == -1
		    || bind(s, (struct sockaddr *)&sa, sizeof(sa)) == -1
		    || connect(s, (struct sockaddr *)&sa, sizeof(sa)) == -1) {
			close(s);
			return -1;
		}

		if ((*cb)(s, &di, arg) != 0)
			break;
	}

thats all for now :)
iain