Hi Johan,
On Thu, Aug 16, 2012 at 2:25 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> Hi Mikel,
>
> On Thu, Aug 09, 2012, Mikel Astiz wrote:
>> From: Mikel Astiz <mikel.astiz@xxxxxxxxxxxx>
>>
>> Extend the management API with the disconnect reason, as now reported
>> by the Kernel in MGMT_EV_DEVICE_DISCONNECTED.
>> ---
>> Updated userland patch to test the recently submitted Kernel patches regarding ACL disconnect reason.
>>
>> doc/mgmt-api.txt | 16 ++++++++++++++++
>> lib/mgmt.h | 6 ++++++
>> monitor/control.c | 19 +++++++++++++++----
>> src/mgmt.c | 13 ++++++++-----
>> 4 files changed, 45 insertions(+), 9 deletions(-)
>
> Seems like you're missing an update to tools/btmgmt.c?
Indeed. I will integrate the changes in the next version of the patch.
>> --- a/monitor/control.c
>> +++ b/monitor/control.c
>> @@ -226,18 +226,29 @@ static void mgmt_device_disconnected(uint16_t len, const void *buf)
>> {
>> const struct mgmt_ev_device_disconnected *ev = buf;
>> char str[18];
>> + uint8_t reason;
>> + uint16_t l;
>
> I suppose size_t would be more appropriate instead of uint16_t as that's
> the return type of sizeof(). The variable name is also a bit
> uninformative and I'd have used something like consumed_len or
> parsed_len, but if this the convention in rest of the btmon code then I
> wont object to it.
I will rename the variable to consumed_len but I would rather leave it
as uint16_t in order to match the type of len.
>> --- a/src/mgmt.c
>> +++ b/src/mgmt.c
>> @@ -510,18 +510,21 @@ static void mgmt_device_connected(int sk, uint16_t index, void *buf, size_t len)
>> static void mgmt_device_disconnected(int sk, uint16_t index, void *buf,
>> size_t len)
>> {
>> - struct mgmt_addr_info *ev = buf;
>> + struct mgmt_ev_device_disconnected *ev = buf;
>> struct controller_info *info;
>> char addr[18];
>>
>> - if (len < sizeof(*ev)) {
>> + if (len < sizeof(struct mgmt_addr_info)) {
>> error("Too small device_disconnected event");
>> return;
>> }
>>
>> - ba2str(&ev->bdaddr, addr);
>> + if (len < sizeof(*ev))
>> + memset((char *) buf + len, 0, sizeof(*ev) - len);
>
> I don't like how this assumes that there is usable space after the end
> of the buffer given to this function. Could you instead use a temporary
> "uint8_t reason" variable like you do for btmon?
I will change this one too.
Cheers,
Mikel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
