* [RFC] mgmt: Add support for Passkey handling @ 2011-12-05 11:46 Hemant Gupta 2011-12-05 12:22 ` Andrei Emeltchenko 2011-12-05 12:50 ` Hendrik Sattler 0 siblings, 2 replies; 7+ messages in thread From: Hemant Gupta @ 2011-12-05 11:46 UTC (permalink / raw) To: linux-bluetooth; +Cc: Naresh Gupta, Hemant Gupta, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC] mgmt: Add support for Passkey handling 2011-12-05 11:46 [RFC] mgmt: Add support for Passkey handling Hemant Gupta @ 2011-12-05 12:22 ` Andrei Emeltchenko [not found] ` <CACj007=e6RLZEJyi4H28TPSjsZEQk_O7suU42U+z9_PeNezsjA@mail.gmail.com> 2011-12-05 12:50 ` Hendrik Sattler 1 sibling, 1 reply; 7+ messages in thread From: Andrei Emeltchenko @ 2011-12-05 12:22 UTC (permalink / raw) To: Hemant Gupta; +Cc: linux-bluetooth, Naresh Gupta, Hemant Gupta 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CACj007=e6RLZEJyi4H28TPSjsZEQk_O7suU42U+z9_PeNezsjA@mail.gmail.com>]
* Re: [RFC] mgmt: Add support for Passkey handling [not found] ` <CACj007=e6RLZEJyi4H28TPSjsZEQk_O7suU42U+z9_PeNezsjA@mail.gmail.com> @ 2011-12-05 16:26 ` Andrei Emeltchenko 0 siblings, 0 replies; 7+ messages in thread From: Andrei Emeltchenko @ 2011-12-05 16:26 UTC (permalink / raw) To: Hemant Gupta; +Cc: Hemant Gupta, linux-bluetooth, Naresh Gupta 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] mgmt: Add support for Passkey handling 2011-12-05 11:46 [RFC] mgmt: Add support for Passkey handling Hemant Gupta 2011-12-05 12:22 ` Andrei Emeltchenko @ 2011-12-05 12:50 ` Hendrik Sattler 2011-12-05 18:59 ` Brian Gix [not found] ` <CACj007npMC+6EWOyNS24o1MvHBetjSXaiXc0HO0c_Z_dZgQADA@mail.gmail.com> 1 sibling, 2 replies; 7+ messages in thread From: Hendrik Sattler @ 2011-12-05 12:50 UTC (permalink / raw) To: Hemant Gupta; +Cc: linux-bluetooth, Naresh Gupta, Hemant Gupta 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) > { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] mgmt: Add support for Passkey handling 2011-12-05 12:50 ` Hendrik Sattler @ 2011-12-05 18:59 ` Brian Gix 2011-12-06 7:43 ` Hemant GUPTA [not found] ` <CACj007npMC+6EWOyNS24o1MvHBetjSXaiXc0HO0c_Z_dZgQADA@mail.gmail.com> 1 sibling, 1 reply; 7+ messages in thread From: Brian Gix @ 2011-12-05 18:59 UTC (permalink / raw) To: Hendrik Sattler; +Cc: Hemant Gupta, linux-bluetooth, Naresh Gupta, Hemant Gupta 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [RFC] mgmt: Add support for Passkey handling 2011-12-05 18:59 ` Brian Gix @ 2011-12-06 7:43 ` Hemant GUPTA 0 siblings, 0 replies; 7+ messages in thread From: Hemant GUPTA @ 2011-12-06 7:43 UTC (permalink / raw) To: Brian Gix, Hendrik Sattler Cc: linux-bluetooth@vger.kernel.org, Naresh-kumar GUPTA, 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CACj007npMC+6EWOyNS24o1MvHBetjSXaiXc0HO0c_Z_dZgQADA@mail.gmail.com>]
* Re: [RFC] mgmt: Add support for Passkey handling [not found] ` <CACj007npMC+6EWOyNS24o1MvHBetjSXaiXc0HO0c_Z_dZgQADA@mail.gmail.com> @ 2011-12-06 7:58 ` Hendrik Sattler 0 siblings, 0 replies; 7+ messages in thread From: Hendrik Sattler @ 2011-12-06 7:58 UTC (permalink / raw) To: Hemant Gupta; +Cc: Hemant Gupta, linux-bluetooth, Naresh Gupta 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-12-06 7:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-05 11:46 [RFC] mgmt: Add support for Passkey handling Hemant Gupta
2011-12-05 12:22 ` Andrei Emeltchenko
[not found] ` <CACj007=e6RLZEJyi4H28TPSjsZEQk_O7suU42U+z9_PeNezsjA@mail.gmail.com>
2011-12-05 16:26 ` Andrei Emeltchenko
2011-12-05 12:50 ` Hendrik Sattler
2011-12-05 18:59 ` Brian Gix
2011-12-06 7:43 ` Hemant GUPTA
[not found] ` <CACj007npMC+6EWOyNS24o1MvHBetjSXaiXc0HO0c_Z_dZgQADA@mail.gmail.com>
2011-12-06 7:58 ` Hendrik Sattler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).