Return-Path: Message-ID: <556253B5.1080403@gmail.com> Date: Mon, 25 May 2015 00:41:57 +0200 From: Florian Grandel MIME-Version: 1.0 To: Marcel Holtmann CC: linux-bluetooth@vger.kernel.org Subject: Re: [PATCH v4 04/17] Bluetooth: mgmt: multi adv for read_adv_features() References: <1430408000-17785-1-git-send-email-fgrandel@gmail.com> <1428633041-18415-1-git-send-email-fgrandel@gmail.com> <1430408000-17785-5-git-send-email-fgrandel@gmail.com> <9016678D-1C7A-4B47-9A78-2AD3D6FB70A3@holtmann.org> In-Reply-To: <9016678D-1C7A-4B47-9A78-2AD3D6FB70A3@holtmann.org> Content-Type: text/plain; charset=windows-1252; format=flowed List-ID: On 05/23/2015 11:25 PM, Marcel Holtmann wrote: > Hi Florian, > >> The read_adv_features() method had a single instance identifier hard >> coded. Refer to the advertising instance list instead to return a >> dynamically generated list of instance identifiers. >> >> Signed-off-by: Florian Grandel >> --- >> net/bluetooth/mgmt.c | 21 +++++++++------------ >> 1 file changed, 9 insertions(+), 12 deletions(-) >> >> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c >> index 9dc4905..55ab2a3 100644 >> --- a/net/bluetooth/mgmt.c >> +++ b/net/bluetooth/mgmt.c >> @@ -6770,8 +6770,9 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev, >> { >> struct mgmt_rp_read_adv_features *rp; >> size_t rp_len; >> - int err; >> + int err, i; >> bool instance; >> + struct adv_info *adv_instance; >> u32 supported_flags; >> >> BT_DBG("%s", hdev->name); >> @@ -6784,12 +6785,9 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev, >> >> rp_len = sizeof(*rp); >> >> - /* Currently only one instance is supported, so just add 1 to the >> - * response length. >> - */ >> instance = hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE); >> if (instance) >> - rp_len++; >> + rp_len += hdev->adv_instance_cnt; >> >> rp = kmalloc(rp_len, GFP_ATOMIC); >> if (!rp) { >> @@ -6803,15 +6801,14 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev, >> rp->max_adv_data_len = HCI_MAX_AD_LENGTH; >> rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH; >> rp->max_instances = HCI_MAX_ADV_INSTANCES; >> + rp->num_instances = hdev->adv_instance_cnt; >> >> - /* Currently only one instance is supported, so simply return the >> - * current instance number. >> - */ >> if (instance) { >> - rp->num_instances = 1; >> - rp->instance[0] = 1; >> - } else { >> - rp->num_instances = 0; >> + i = 0; >> + list_for_each_entry(adv_instance, &hdev->adv_instances, list) { >> + rp->instance[i] = adv_instance->instance; >> + i++; >> + } > > I would really prefer that we leave this logic in place. When !instance, then keep returning rp->num_instances = 0 and in the other case set it to hdev->adv_instance_cnt. I would also prefer that the list is really limited by adv_instance_cnt and not blindly trusted that the list size is the same. Done... see the latest patch set (v5). > > Regards > > Marcel > > -- > 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 > Florian