Hi Jakub,
>>> On Sat, Mar 21, 2015 at 3:15 PM, Jakub Pawlowski <jpawlowski@xxxxxxxxxx> wrote:
>>> This patch replaces scan type with defined constants, and creates
>>> new method that might be used to get currently avaliable scan type.
>>> ---
>>> src/adapter.c | 29 +++++++++++++++++++++--------
>>> 1 file changed, 21 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/adapter.c b/src/adapter.c
>>> index 6eeb2f9..99f9786 100644
>>> --- a/src/adapter.c
>>> +++ b/src/adapter.c
>>> @@ -88,6 +88,10 @@
>>> #define TEMP_DEV_TIMEOUT (3 * 60)
>>> #define BONDING_TIMEOUT (2 * 60)
>>>
>>> +#define SCAN_TYPE_BREDR (1 << BDADDR_BREDR)
>>> +#define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
>>> +#define SCAN_TYPE_DUAL (SCAN_TYPE_BREDR | SCAN_TYPE_LE)
>>
>> SCAN_TYPE_DUAL isn't being used anywhere in this patch, is it needed
>> here? Maybe only include this in the patch that actually uses it?
>>
> I'll fix that.
>>> +
>>> static DBusConnection *dbus_conn = NULL;
>>>
>>> static bool kernel_conn_control = false;
>>> @@ -1185,7 +1189,7 @@ static gboolean passive_scanning_timeout(gpointer user_data)
>>>
>>> adapter->passive_scan_timeout = 0;
>>>
>>> - cp.type = (1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM);
>>> + cp.type = SCAN_TYPE_LE;
>>>
>>> mgmt_send(adapter->mgmt, MGMT_OP_START_DISCOVERY,
>>> adapter->dev_id, sizeof(cp), &cp,
>>> @@ -1327,6 +1331,21 @@ static void cancel_passive_scanning(struct btd_adapter *adapter)
>>> }
>>> }
>>>
>>> +static uint8_t get_current_type(struct btd_adapter *adapter)
>>> +{
>>> + uint8_t type;
>>> +
>>> + if (adapter->current_settings & MGMT_SETTING_BREDR)
>>> + type = SCAN_TYPE_BREDR;
>>> + else
>>> + type = 0;
>>
>> I'd just initialize type = 0 and then bitwise or SCAN_TYPE_BREDR if
>> the BREDR setting is set, which I think makes for more readable code.
>>
>
> So when I was writing some kernel code, Marcel told me that they
> prefer to delay initialization of variables as long as possible, and
> that's in that manner. I also didn't write this code, I just moved it
> from other place, so this should be good, or was some convention
> changed ?
this rule is still in affect and we want it. Mainly so that the compiler will warn us when we accidentally have a new code path. I want people to think about their code and if the compiler just complains, that is normally a good indication that something needs some extra look at it.
Regards
Marcel
--
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
