Hi Florian,
On Sun, Apr 05, 2015, Florian Grandel wrote:
> As a preparatory step towards multi-instance advertising it is
> necessary to introduce a data structure that supports storing
> multiple advertising info data structures for a bluetooth device.
>
> This is introduced by refactoring the existing adv_instance member
> of the hci_dev struct into a linked list and making the necessary
> changes in the code to support this list.
>
> Signed-off-by: Florian Grandel <fgrandel@xxxxxxxxx>
> ---
>
> v1 -> v2: add missing braces in read_adv_features()
>
> include/net/bluetooth/hci_core.h | 21 ++-
> net/bluetooth/hci_core.c | 118 ++++++++++++-
> net/bluetooth/mgmt.c | 345 +++++++++++++++++++++------------------
> 3 files changed, 316 insertions(+), 168 deletions(-)
It'd be really good if you could try to split this patch up a bit. It
really is going over the limits of individual patch sizes that we're
comfortable with. See if you can split this up logically into at least
two, preferably more (3-4) patches while still keeping something that
compiles during each step.
> + if (err == 0)
> + advertising_removed(sk, hdev,
> + adv_instance->instance);
> + }
> + } else {
> + err = hci_remove_adv_instance(hdev, instance);
> + if (err == 0)
> + advertising_removed(sk, hdev, instance);
I think if (!err) is the more consistent convension.
> + err = hci_add_adv_instance(hdev, cp->instance, flags,
> + cp->adv_data_len, cp->data,
> + cp->scan_rsp_len,
> + cp->data + cp->adv_data_len,
> + adv_timeout_expired, timeout);
The split lines don't look like they're correctly lined up. They should
start from one column after the opening '(' on the first line.
> + // TODO: Trigger an advertising added event even when instance
> + // advertising is already switched on?
Please stick to /* ... */ for comments.
Johan
--
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
