[RFC] mgmt: Add support for Passkey handling

[RFC] mgmt: Add support for Passkey handling

[Hemant Gupta]

This patch adds support for handling Passkey Requests
and response over management interface.
---
 lib/mgmt.h        |   17 ++++++++++
 plugins/mgmtops.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 93 insertions(+), 10 deletions(-)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 3960815..a85957d 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -242,6 +242,17 @@ struct mgmt_cp_set_fast_connectable {
 	uint8_t enable;
 } __packed;
 
+#define MGMT_OP_USER_PASSKEY_REPLY	0x0020
+struct mgmt_cp_user_passkey_reply {
+	bdaddr_t bdaddr;
+	__le32 passkey;
+} __packed;
+
+#define MGMT_OP_USER_PASSKEY_NEG_REPLY	0x0021
+struct mgmt_cp_user_passkey_neg_reply {
+	bdaddr_t bdaddr;
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	uint16_t opcode;
@@ -336,3 +347,9 @@ struct mgmt_ev_device_blocked {
 struct mgmt_ev_device_unblocked {
 	bdaddr_t bdaddr;
 } __packed;
+
+#define MGMT_EV_USER_PASSKEY_REQUEST	0x0017
+struct mgmt_ev_user_passkey_request {
+	bdaddr_t bdaddr;
+} __packed;
+
diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index b9e9ad6..ef88ae6 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -615,6 +615,79 @@ static int mgmt_confirm_reply(int index, bdaddr_t *bdaddr, gboolean success)
 	return 0;
 }
 
+static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t passkey)
+{
+	char buf[MGMT_HDR_SIZE + sizeof(struct mgmt_cp_user_passkey_reply)];
+	struct mgmt_hdr *hdr = (void *) buf;
+	size_t buf_len;
+	char addr[18];
+
+	ba2str(bdaddr, addr);
+	DBG("index %d addr %s passkey %06u", index, addr, passkey);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (passkey == INVALID_PASSKEY) {
+		struct mgmt_cp_user_passkey_neg_reply *cp;
+
+		hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
+		hdr->len = htobs(sizeof(*cp));
+		hdr->index = htobs(index);
+
+		cp = (void *) &buf[sizeof(*hdr)];
+		bacpy(&cp->bdaddr, bdaddr);
+
+		buf_len = sizeof(*hdr) + sizeof(*cp);
+	} else {
+		struct mgmt_cp_user_passkey_reply *cp;
+
+		hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_REPLY);
+		hdr->len = htobs(sizeof(*cp));
+		hdr->index = htobs(index);
+
+		cp = (void *) &buf[sizeof(*hdr)];
+		bacpy(&cp->bdaddr, bdaddr);
+		cp->passkey = htobl(passkey);
+
+		buf_len = sizeof(*hdr) + sizeof(*cp);
+	}
+
+	if (write(mgmt_sock, buf, buf_len) < 0)
+		return -errno;
+
+	return 0;
+}
+
+static void mgmt_passkey_request(int sk, uint16_t index, void *buf, size_t len)
+{
+	struct mgmt_ev_user_passkey_request *ev = buf;
+	struct controller_info *info;
+	char addr[18];
+	int err;
+
+	if (len < sizeof(*ev)) {
+		error("Too small pin_code_request event");
+		return;
+	}
+
+	ba2str(&ev->bdaddr, addr);
+
+	DBG("hci%u %s", index, addr);
+
+	if (index > max_index) {
+		error("Unexpected index %u in passkey_request event", index);
+		return;
+	}
+
+	info = &controllers[index];
+
+	err = btd_event_user_passkey(&info->bdaddr, &ev->bdaddr);
+	if (err < 0) {
+		error("btd_event_request_pin: %s", strerror(-err));
+		mgmt_passkey_reply(index, &ev->bdaddr, INVALID_PASSKEY);
+	}
+}
+
 struct confirm_data {
 	int index;
 	bdaddr_t bdaddr;
@@ -1576,6 +1649,9 @@ static gboolean mgmt_event(GIOChannel *io, GIOCondition cond, gpointer user_data
 	case MGMT_EV_DEVICE_UNBLOCKED:
 		mgmt_device_unblocked(sk, index, buf + MGMT_HDR_SIZE, len);
 		break;
+	case MGMT_EV_USER_PASSKEY_REQUEST:
+		mgmt_passkey_request(sk, index, buf + MGMT_HDR_SIZE, len);
+		break;
 	default:
 		error("Unknown Management opcode %u (index %u)", opcode, index);
 		break;
@@ -1919,16 +1995,6 @@ static int mgmt_remove_bonding(int index, bdaddr_t *bdaddr)
 	return 0;
 }
 
-static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t passkey)
-{
-	char addr[18];
-
-	ba2str(bdaddr, addr);
-	DBG("index %d addr %s passkey %06u", index, addr, passkey);
-
-	return -ENOSYS;
-}
-
 static int mgmt_encrypt_link(int index, bdaddr_t *dst, bt_hci_result_t cb,
 							gpointer user_data)
 {
-- 
1.6.6.1

Re: [RFC] mgmt: Add support for Passkey handling

[Andrei Emeltchenko]

Hi Hemant,

On Mon, Dec 05, 2011 at 05:16:26PM +0530, Hemant Gupta wrote:
> This patch adds support for handling Passkey Requests
> and response over management interface.
> ---
>  lib/mgmt.h        |   17 ++++++++++
>  plugins/mgmtops.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 93 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 3960815..a85957d 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -242,6 +242,17 @@ struct mgmt_cp_set_fast_connectable {
>  	uint8_t enable;
>  } __packed;
>  
> +#define MGMT_OP_USER_PASSKEY_REPLY	0x0020
> +struct mgmt_cp_user_passkey_reply {
> +	bdaddr_t bdaddr;
> +	__le32 passkey;
> +} __packed;
> +
> +#define MGMT_OP_USER_PASSKEY_NEG_REPLY	0x0021
> +struct mgmt_cp_user_passkey_neg_reply {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
>  #define MGMT_EV_CMD_COMPLETE		0x0001
>  struct mgmt_ev_cmd_complete {
>  	uint16_t opcode;
> @@ -336,3 +347,9 @@ struct mgmt_ev_device_blocked {
>  struct mgmt_ev_device_unblocked {
>  	bdaddr_t bdaddr;
>  } __packed;
> +
> +#define MGMT_EV_USER_PASSKEY_REQUEST	0x0017
> +struct mgmt_ev_user_passkey_request {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
> diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
> index b9e9ad6..ef88ae6 100644
> --- a/plugins/mgmtops.c
> +++ b/plugins/mgmtops.c
> @@ -615,6 +615,79 @@ static int mgmt_confirm_reply(int index, bdaddr_t *bdaddr, gboolean success)
>  	return 0;
>  }
>  
> +static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t passkey)
> +{
> +	char buf[MGMT_HDR_SIZE + sizeof(struct mgmt_cp_user_passkey_reply)];
> +	struct mgmt_hdr *hdr = (void *) buf;
> +	size_t buf_len;
> +	char addr[18];
> +
> +	ba2str(bdaddr, addr);
> +	DBG("index %d addr %s passkey %06u", index, addr, passkey);

isn't it too much overhead to fill in (and to have) buffer even when debug disabled?

> +
> +	memset(buf, 0, sizeof(buf));
> +
> +	if (passkey == INVALID_PASSKEY) {
> +		struct mgmt_cp_user_passkey_neg_reply *cp;
> +
> +		hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
> +		hdr->len = htobs(sizeof(*cp));
> +		hdr->index = htobs(index);
> +
> +		cp = (void *) &buf[sizeof(*hdr)];

The code above is a cool hackers code :-)

Best regards 
Andrei Emeltchenko 

> +		bacpy(&cp->bdaddr, bdaddr);
> +
> +		buf_len = sizeof(*hdr) + sizeof(*cp);
> +	} else {
> +		struct mgmt_cp_user_passkey_reply *cp;
> +
> +		hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_REPLY);
> +		hdr->len = htobs(sizeof(*cp));
> +		hdr->index = htobs(index);
> +
> +		cp = (void *) &buf[sizeof(*hdr)];
> +		bacpy(&cp->bdaddr, bdaddr);
> +		cp->passkey = htobl(passkey);
> +
> +		buf_len = sizeof(*hdr) + sizeof(*cp);
> +	}
> +
> +	if (write(mgmt_sock, buf, buf_len) < 0)
> +		return -errno;
> +
> +	return 0;
> +}
> +
> +static void mgmt_passkey_request(int sk, uint16_t index, void *buf, size_t len)
> +{
> +	struct mgmt_ev_user_passkey_request *ev = buf;
> +	struct controller_info *info;
> +	char addr[18];
> +	int err;
> +
> +	if (len < sizeof(*ev)) {
> +		error("Too small pin_code_request event");
> +		return;
> +	}
> +
> +	ba2str(&ev->bdaddr, addr);
> +
> +	DBG("hci%u %s", index, addr);
> +
> +	if (index > max_index) {
> +		error("Unexpected index %u in passkey_request event", index);
> +		return;
> +	}
> +
> +	info = &controllers[index];
> +
> +	err = btd_event_user_passkey(&info->bdaddr, &ev->bdaddr);
> +	if (err < 0) {
> +		error("btd_event_request_pin: %s", strerror(-err));
> +		mgmt_passkey_reply(index, &ev->bdaddr, INVALID_PASSKEY);
> +	}
> +}
> +
>  struct confirm_data {
>  	int index;
>  	bdaddr_t bdaddr;
> @@ -1576,6 +1649,9 @@ static gboolean mgmt_event(GIOChannel *io, GIOCondition cond, gpointer user_data
>  	case MGMT_EV_DEVICE_UNBLOCKED:
>  		mgmt_device_unblocked(sk, index, buf + MGMT_HDR_SIZE, len);
>  		break;
> +	case MGMT_EV_USER_PASSKEY_REQUEST:
> +		mgmt_passkey_request(sk, index, buf + MGMT_HDR_SIZE, len);
> +		break;
>  	default:
>  		error("Unknown Management opcode %u (index %u)", opcode, index);
>  		break;
> @@ -1919,16 +1995,6 @@ static int mgmt_remove_bonding(int index, bdaddr_t *bdaddr)
>  	return 0;
>  }
>  
> -static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t passkey)
> -{
> -	char addr[18];
> -
> -	ba2str(bdaddr, addr);
> -	DBG("index %d addr %s passkey %06u", index, addr, passkey);
> -
> -	return -ENOSYS;
> -}
> -
>  static int mgmt_encrypt_link(int index, bdaddr_t *dst, bt_hci_result_t cb,
>  							gpointer user_data)
>  {
> -- 
> 1.6.6.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] mgmt: Add support for Passkey handling

[Hendrik Sattler]

Am 05.12.2011 12:46, schrieb Hemant Gupta:
> This patch adds support for handling Passkey Requests
> and response over management interface.
> ---
>  lib/mgmt.h        |   17 ++++++++++
>  plugins/mgmtops.c |   86
> ++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 93 insertions(+), 10 deletions(-)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 3960815..a85957d 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -242,6 +242,17 @@ struct mgmt_cp_set_fast_connectable {
>  	uint8_t enable;
>  } __packed;
>
> +#define MGMT_OP_USER_PASSKEY_REPLY	0x0020
> +struct mgmt_cp_user_passkey_reply {
> +	bdaddr_t bdaddr;
> +	__le32 passkey;
> +} __packed;
> +
> +#define MGMT_OP_USER_PASSKEY_NEG_REPLY	0x0021
> +struct mgmt_cp_user_passkey_neg_reply {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
>  #define MGMT_EV_CMD_COMPLETE		0x0001
>  struct mgmt_ev_cmd_complete {
>  	uint16_t opcode;
> @@ -336,3 +347,9 @@ struct mgmt_ev_device_blocked {
>  struct mgmt_ev_device_unblocked {
>  	bdaddr_t bdaddr;
>  } __packed;
> +
> +#define MGMT_EV_USER_PASSKEY_REQUEST	0x0017
> +struct mgmt_ev_user_passkey_request {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
> diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
> index b9e9ad6..ef88ae6 100644
> --- a/plugins/mgmtops.c
> +++ b/plugins/mgmtops.c
> @@ -615,6 +615,79 @@ static int mgmt_confirm_reply(int index,
> bdaddr_t *bdaddr, gboolean success)
>  	return 0;
>  }
>
> +static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t 
> passkey)
> +{
> +	char buf[MGMT_HDR_SIZE + sizeof(struct 
> mgmt_cp_user_passkey_reply)];
> +	struct mgmt_hdr *hdr = (void *) buf;
> +	size_t buf_len;
> +	char addr[18];
> +
> +	ba2str(bdaddr, addr);
> +	DBG("index %d addr %s passkey %06u", index, addr, passkey);
> +
> +	memset(buf, 0, sizeof(buf));
> +
> +	if (passkey == INVALID_PASSKEY) {
> +		struct mgmt_cp_user_passkey_neg_reply *cp;
> +
> +		hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
> +		hdr->len = htobs(sizeof(*cp));
> +		hdr->index = htobs(index);
> +
> +		cp = (void *) &buf[sizeof(*hdr)];

By definition, that the same as:
    cp = (void *) (hdr + 1);
And you can do it in the same line as the definition of *cp.

> +		bacpy(&cp->bdaddr, bdaddr);
> +
> +		buf_len = sizeof(*hdr) + sizeof(*cp);
> +	} else {
> +		struct mgmt_cp_user_passkey_reply *cp;
> +
> +		hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_REPLY);
> +		hdr->len = htobs(sizeof(*cp));
> +		hdr->index = htobs(index);
> +
> +		cp = (void *) &buf[sizeof(*hdr)];
> +		bacpy(&cp->bdaddr, bdaddr);
> +		cp->passkey = htobl(passkey);
> +
> +		buf_len = sizeof(*hdr) + sizeof(*cp);
> +	}

Ever wondered why if and else look almost the same? They should be 
merged as far as possible.

> +	if (write(mgmt_sock, buf, buf_len) < 0)
> +		return -errno;
> +
> +	return 0;
> +}
> +
> +static void mgmt_passkey_request(int sk, uint16_t index, void *buf,
> size_t len)
> +{
> +	struct mgmt_ev_user_passkey_request *ev = buf;
> +	struct controller_info *info;
> +	char addr[18];
> +	int err;
> +
> +	if (len < sizeof(*ev)) {
> +		error("Too small pin_code_request event");
> +		return;
> +	}
> +
> +	ba2str(&ev->bdaddr, addr);
> +
> +	DBG("hci%u %s", index, addr);
> +
> +	if (index > max_index) {
> +		error("Unexpected index %u in passkey_request event", index);
> +		return;
> +	}
> +
> +	info = &controllers[index];
> +
> +	err = btd_event_user_passkey(&info->bdaddr, &ev->bdaddr);
> +	if (err < 0) {
> +		error("btd_event_request_pin: %s", strerror(-err));
> +		mgmt_passkey_reply(index, &ev->bdaddr, INVALID_PASSKEY);
> +	}
> +}
> +
>  struct confirm_data {
>  	int index;
>  	bdaddr_t bdaddr;
> @@ -1576,6 +1649,9 @@ static gboolean mgmt_event(GIOChannel *io,
> GIOCondition cond, gpointer user_data
>  	case MGMT_EV_DEVICE_UNBLOCKED:
>  		mgmt_device_unblocked(sk, index, buf + MGMT_HDR_SIZE, len);
>  		break;
> +	case MGMT_EV_USER_PASSKEY_REQUEST:
> +		mgmt_passkey_request(sk, index, buf + MGMT_HDR_SIZE, len);
> +		break;
>  	default:
>  		error("Unknown Management opcode %u (index %u)", opcode, index);
>  		break;
> @@ -1919,16 +1995,6 @@ static int mgmt_remove_bonding(int index,
> bdaddr_t *bdaddr)
>  	return 0;
>  }
>
> -static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t 
> passkey)
> -{
> -	char addr[18];
> -
> -	ba2str(bdaddr, addr);
> -	DBG("index %d addr %s passkey %06u", index, addr, passkey);
> -
> -	return -ENOSYS;
> -}
> -
>  static int mgmt_encrypt_link(int index, bdaddr_t *dst, 
> bt_hci_result_t cb,
>  							gpointer user_data)
>  {

Re: [RFC] mgmt: Add support for Passkey handling

[Andrei Emeltchenko]

Hi Hemant,

On Mon, Dec 05, 2011 at 07:42:00PM +0530, Hemant Gupta wrote:
> Hi Andrei,
> 
> On Mon, Dec 5, 2011 at 5:52 PM, Andrei Emeltchenko <
> andrei.emeltchenko.news@gmail.com> wrote:
> 
> > Hi Hemant,
> >
> > On Mon, Dec 05, 2011 at 05:16:26PM +0530, Hemant Gupta wrote:
> > > This patch adds support for handling Passkey Requests
> > > and response over management interface.
> > > ---
> > >  lib/mgmt.h        |   17 ++++++++++
> > >  plugins/mgmtops.c |   86
> > ++++++++++++++++++++++++++++++++++++++++++++++------
> > >  2 files changed, 93 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/lib/mgmt.h b/lib/mgmt.h
> > > index 3960815..a85957d 100644
> > > --- a/lib/mgmt.h
> > > +++ b/lib/mgmt.h
> > > @@ -242,6 +242,17 @@ struct mgmt_cp_set_fast_connectable {
> > >       uint8_t enable;
> > >  } __packed;
> > >
> > > +#define MGMT_OP_USER_PASSKEY_REPLY   0x0020
> > > +struct mgmt_cp_user_passkey_reply {
> > > +     bdaddr_t bdaddr;
> > > +     __le32 passkey;
> > > +} __packed;
> > > +
> > > +#define MGMT_OP_USER_PASSKEY_NEG_REPLY       0x0021
> > > +struct mgmt_cp_user_passkey_neg_reply {
> > > +     bdaddr_t bdaddr;
> > > +} __packed;
> > > +
> > >  #define MGMT_EV_CMD_COMPLETE         0x0001
> > >  struct mgmt_ev_cmd_complete {
> > >       uint16_t opcode;
> > > @@ -336,3 +347,9 @@ struct mgmt_ev_device_blocked {
> > >  struct mgmt_ev_device_unblocked {
> > >       bdaddr_t bdaddr;
> > >  } __packed;
> > > +
> > > +#define MGMT_EV_USER_PASSKEY_REQUEST 0x0017
> > > +struct mgmt_ev_user_passkey_request {
> > > +     bdaddr_t bdaddr;
> > > +} __packed;
> > > +
> > > diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
> > > index b9e9ad6..ef88ae6 100644
> > > --- a/plugins/mgmtops.c
> > > +++ b/plugins/mgmtops.c
> > > @@ -615,6 +615,79 @@ static int mgmt_confirm_reply(int index, bdaddr_t
> > *bdaddr, gboolean success)
> > >       return 0;
> > >  }
> > >
> > > +static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t
> > passkey)
> > > +{
> > > +     char buf[MGMT_HDR_SIZE + sizeof(struct
> > mgmt_cp_user_passkey_reply)];
> > > +     struct mgmt_hdr *hdr = (void *) buf;
> > > +     size_t buf_len;
> > > +     char addr[18];
> > > +
> > > +     ba2str(bdaddr, addr);
> > > +     DBG("index %d addr %s passkey %06u", index, addr, passkey);
> >
> > isn't it too much overhead to fill in (and to have) buffer even when debug
> > disabled?
> 
> 
> I have tried to re-use the exisitng implementation in mgmtops.c. If you
> look at the implementation of mgmt_pincode_reply(), you would find the
> similar implementation. Are you suggesting that I should change the
> existing implementation also, and prepare a patch accordingly or only
> change the implementation in this API ?

I think you can start from your implementation and then create patch
against other cases. Johan could comment further here.

Best regards 
Andrei Emeltchenko 


> 
> 
> > > +
> > > +     memset(buf, 0, sizeof(buf));
> > > +
> > > +     if (passkey == INVALID_PASSKEY) {
> > > +             struct mgmt_cp_user_passkey_neg_reply *cp;
> > > +
> > > +             hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
> > > +             hdr->len = htobs(sizeof(*cp));
> > > +             hdr->index = htobs(index);
> > > +
> > > +             cp = (void *) &buf[sizeof(*hdr)];
> >
> > The code above is a cool hackers code :-)
> >
> > Best regards
> > Andrei Emeltchenko
> >
> > > +             bacpy(&cp->bdaddr, bdaddr);
> > > +
> > > +             buf_len = sizeof(*hdr) + sizeof(*cp);
> > > +     } else {
> > > +             struct mgmt_cp_user_passkey_reply *cp;
> > > +
> > > +             hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_REPLY);
> > > +             hdr->len = htobs(sizeof(*cp));
> > > +             hdr->index = htobs(index);
> > > +
> > > +             cp = (void *) &buf[sizeof(*hdr)];
> > > +             bacpy(&cp->bdaddr, bdaddr);
> > > +             cp->passkey = htobl(passkey);
> > > +
> > > +             buf_len = sizeof(*hdr) + sizeof(*cp);
> > > +     }
> > > +
> > > +     if (write(mgmt_sock, buf, buf_len) < 0)
> > > +             return -errno;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static void mgmt_passkey_request(int sk, uint16_t index, void *buf,
> > size_t len)
> > > +{
> > > +     struct mgmt_ev_user_passkey_request *ev = buf;
> > > +     struct controller_info *info;
> > > +     char addr[18];
> > > +     int err;
> > > +
> > > +     if (len < sizeof(*ev)) {
> > > +             error("Too small pin_code_request event");
> > > +             return;
> > > +     }
> > > +
> > > +     ba2str(&ev->bdaddr, addr);
> > > +
> > > +     DBG("hci%u %s", index, addr);
> > > +
> > > +     if (index > max_index) {
> > > +             error("Unexpected index %u in passkey_request event",
> > index);
> > > +             return;
> > > +     }
> > > +
> > > +     info = &controllers[index];
> > > +
> > > +     err = btd_event_user_passkey(&info->bdaddr, &ev->bdaddr);
> > > +     if (err < 0) {
> > > +             error("btd_event_request_pin: %s", strerror(-err));
> > > +             mgmt_passkey_reply(index, &ev->bdaddr, INVALID_PASSKEY);
> > > +     }
> > > +}
> > > +
> > >  struct confirm_data {
> > >       int index;
> > >       bdaddr_t bdaddr;
> > > @@ -1576,6 +1649,9 @@ static gboolean mgmt_event(GIOChannel *io,
> > GIOCondition cond, gpointer user_data
> > >       case MGMT_EV_DEVICE_UNBLOCKED:
> > >               mgmt_device_unblocked(sk, index, buf + MGMT_HDR_SIZE, len);
> > >               break;
> > > +     case MGMT_EV_USER_PASSKEY_REQUEST:
> > > +             mgmt_passkey_request(sk, index, buf + MGMT_HDR_SIZE, len);
> > > +             break;
> > >       default:
> > >               error("Unknown Management opcode %u (index %u)", opcode,
> > index);
> > >               break;
> > > @@ -1919,16 +1995,6 @@ static int mgmt_remove_bonding(int index,
> > bdaddr_t *bdaddr)
> > >       return 0;
> > >  }
> > >
> > > -static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t
> > passkey)
> > > -{
> > > -     char addr[18];
> > > -
> > > -     ba2str(bdaddr, addr);
> > > -     DBG("index %d addr %s passkey %06u", index, addr, passkey);
> > > -
> > > -     return -ENOSYS;
> > > -}
> > > -
> > >  static int mgmt_encrypt_link(int index, bdaddr_t *dst, bt_hci_result_t
> > cb,
> > >                                                       gpointer user_data)
> > >  {
> > > --
> > > 1.6.6.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> 
> 
> -- 
> Best Regards
> Hemant Gupta
> ST-Ericsson India

Re: [RFC] mgmt: Add support for Passkey handling

[Brian Gix]

Hi Hendrik,

On 12/5/2011 4:50 AM, Hendrik Sattler wrote:
> Am 05.12.2011 12:46, schrieb Hemant Gupta:
[...]
>> +static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t
>> passkey)
>> +{
>> + char buf[MGMT_HDR_SIZE + sizeof(struct mgmt_cp_user_passkey_reply)];
>> + struct mgmt_hdr *hdr = (void *) buf;
>> + size_t buf_len;
>> + char addr[18];
>> +
>> + ba2str(bdaddr, addr);
>> + DBG("index %d addr %s passkey %06u", index, addr, passkey);
>> +
>> + memset(buf, 0, sizeof(buf));
>> +
>> + if (passkey == INVALID_PASSKEY) {
>> + struct mgmt_cp_user_passkey_neg_reply *cp;
>> +
>> + hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
>> + hdr->len = htobs(sizeof(*cp));
>> + hdr->index = htobs(index);
>> +
>> + cp = (void *) &buf[sizeof(*hdr)];
>
> By definition, that the same as:
> cp = (void *) (hdr + 1);
> And you can do it in the same line as the definition of *cp.
>
>> + bacpy(&cp->bdaddr, bdaddr);
>> +
>> + buf_len = sizeof(*hdr) + sizeof(*cp);
>> + } else {
>> + struct mgmt_cp_user_passkey_reply *cp;
>> +
>> + hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_REPLY);
>> + hdr->len = htobs(sizeof(*cp));
>> + hdr->index = htobs(index);
>> +
>> + cp = (void *) &buf[sizeof(*hdr)];
>> + bacpy(&cp->bdaddr, bdaddr);
>> + cp->passkey = htobl(passkey);
>> +
>> + buf_len = sizeof(*hdr) + sizeof(*cp);
>> + }
>
> Ever wondered why if and else look almost the same? They should be
> merged as far as possible.

I generally agree with this statement, but think it doesn't really apply 
here.  Since the definition of "cp" is different between the IF and the 
ELSE, the generated code is quite different, even though they look the 
same:  The sizeof(*cp) is different, the MGMT_OP Opcode is different, 
and the structure members might be named the same, but since they are in 
different structures, they are in fact different as well.

The only thing that could be safely "shared" is the hdr->index, I think.


-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

RE: [RFC] mgmt: Add support for Passkey handling

[Hemant GUPTA]

Hi Brian, Hendrik,

-----Original Message-----
From: Brian Gix [mailto:bgix@codeaurora.org] 
Sent: Tuesday, December 06, 2011 12:29 AM
To: Hendrik Sattler
Cc: Hemant GUPTA; linux-bluetooth@vger.kernel.org; Naresh-kumar GUPTA; Hemant Gupta
Subject: Re: [RFC] mgmt: Add support for Passkey handling

Hi Hendrik,

On 12/5/2011 4:50 AM, Hendrik Sattler wrote:
> Am 05.12.2011 12:46, schrieb Hemant Gupta:
[...]
>> +static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t
>> passkey)
>> +{
>> + char buf[MGMT_HDR_SIZE + sizeof(struct mgmt_cp_user_passkey_reply)];
>> + struct mgmt_hdr *hdr = (void *) buf;
>> + size_t buf_len;
>> + char addr[18];
>> +
>> + ba2str(bdaddr, addr);
>> + DBG("index %d addr %s passkey %06u", index, addr, passkey);
>> +
>> + memset(buf, 0, sizeof(buf));
>> +
>> + if (passkey == INVALID_PASSKEY) {
>> + struct mgmt_cp_user_passkey_neg_reply *cp;
>> +
>> + hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
>> + hdr->len = htobs(sizeof(*cp));
>> + hdr->index = htobs(index);
>> +
>> + cp = (void *) &buf[sizeof(*hdr)];
>
> By definition, that the same as:
> cp = (void *) (hdr + 1);
> And you can do it in the same line as the definition of *cp.
>
>> + bacpy(&cp->bdaddr, bdaddr);
>> +
>> + buf_len = sizeof(*hdr) + sizeof(*cp);
>> + } else {
>> + struct mgmt_cp_user_passkey_reply *cp;
>> +
>> + hdr->opcode = htobs(MGMT_OP_USER_PASSKEY_REPLY);
>> + hdr->len = htobs(sizeof(*cp));
>> + hdr->index = htobs(index);
>> +
>> + cp = (void *) &buf[sizeof(*hdr)];
>> + bacpy(&cp->bdaddr, bdaddr);
>> + cp->passkey = htobl(passkey);
>> +
>> + buf_len = sizeof(*hdr) + sizeof(*cp);
>> + }
>
> Ever wondered why if and else look almost the same? They should be
> merged as far as possible.

I generally agree with this statement, but think it doesn't really apply 
here.  Since the definition of "cp" is different between the IF and the 
ELSE, the generated code is quite different, even though they look the 
same:  The sizeof(*cp) is different, the MGMT_OP Opcode is different, 
and the structure members might be named the same, but since they are in 
different structures, they are in fact different as well.

The only thing that could be safely "shared" is the hdr->index, I think.

I agree with you comments, and would be uploading a new patch for review. 

-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

BR
Hemant Gupta
ST-Ericsson

Re: [RFC] mgmt: Add support for Passkey handling

[Hendrik Sattler]

Am 05.12.2011 15:12, schrieb Hemant Gupta:
> On Mon, Dec 5, 2011 at 6:20 PM, Hendrik Sattler
> <post@hendrik-sattler.de>wrote:
>> Am 05.12.2011 12:46, schrieb Hemant Gupta:
>>> +static int mgmt_passkey_reply(int index, bdaddr_t *bdaddr, 
>>> uint32_t
>>> passkey)
>>> +{
>>> +       char buf[MGMT_HDR_SIZE + sizeof(struct
>>> mgmt_cp_user_passkey_reply)];
>>> +       struct mgmt_hdr *hdr = (void *) buf;
>>> +       size_t buf_len;
>>> +       char addr[18];
>>> +
>>> +       ba2str(bdaddr, addr);
>>> +       DBG("index %d addr %s passkey %06u", index, addr, passkey);
>>> +
>>> +       memset(buf, 0, sizeof(buf));
>>> +
>>> +       if (passkey == INVALID_PASSKEY) {
>>> +               struct mgmt_cp_user_passkey_neg_reply *cp;
>>> +
>>> +               hdr->opcode = 
>>> htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY);
>>> +               hdr->len = htobs(sizeof(*cp));
>>> +               hdr->index = htobs(index);
>>> +
>>> +               cp = (void *) &buf[sizeof(*hdr)];
>>>
>>
>> By definition, that the same as:
>>   cp = (void *) (hdr + 1);
>> And you can do it in the same line as the definition of *cp.
>> I have tried to re-use the exisitng implementation in mgmtops.c. If 
>> you
>> look at the implementation of mgmt_pincode_reply(), you would find 
>> the
>> similar implementation. Are you suggesting that I should change the
>> existing implementation also, and prepare a patch accordingly or 
>> only
>> specific to this API ?

Maybe this is a possible implementation start (in C99):
static int mgmt_passkey_neg_reply(int index, bdaddr_t *bdaddr)
{
     struct {
         struct mgmt_hdr hdr;
         struct mgmt_cp_user_passkey_neg_reply cp;
     } __packed buf = {
         .hdr = {
             .opcode = htobs(MGMT_OP_USER_PASSKEY_NEG_REPLY),
             .len = htobs(sizeof(*buf.cp)),
             .index = htobs(index),
         },
         .cp = {
             ....
         },
     };
     /* write() and return */
     ....
}

No pointer casting or offset calculation, seperate functions for 
neg_reply and reply.
But maybe you are right and this should be not be done in your patch...

HS