Hi Abhishek,
On Fri, Sep 11, 2020 at 3:30 PM Abhishek Pandit-Subedi
<abhishekpandit@xxxxxxxxxxxx> wrote:
>
> Move all the kernel specific feature globals into a single
> kernel_features bitfield and replace all uses with the bitfield instead.
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> src/adapter.c | 59 ++++++++++++++++++++++++++-------------------------
> src/adapter.h | 9 ++++++++
> 2 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 1435e2bd7..e700a78d5 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -116,13 +116,7 @@ static const struct mgmt_blocked_key_info blocked_keys[] = {
>
> static DBusConnection *dbus_conn = NULL;
>
> -static bool kernel_conn_control = false;
> -
> -static bool kernel_blocked_keys_supported = false;
> -
> -static bool kernel_set_system_config = false;
> -
> -static bool kernel_exp_features = false;
> +static uint32_t kernel_features = 0;
>
> static GList *adapter_list = NULL;
> static unsigned int adapter_remaining = 0;
> @@ -678,7 +672,7 @@ static bool set_discoverable(struct btd_adapter *adapter, uint8_t mode,
>
> DBG("sending set mode command for index %u", adapter->dev_id);
>
> - if (kernel_conn_control) {
> + if (has_kernel_features(KERNEL_CONN_CONTROL)) {
> if (mode)
> set_mode(adapter, MGMT_OP_SET_CONNECTABLE, mode);
> else
> @@ -1334,7 +1328,7 @@ static void trigger_passive_scanning(struct btd_adapter *adapter)
> * no need to start any discovery. The kernel will keep scanning
> * as long as devices are in its auto-connection list.
> */
> - if (kernel_conn_control)
> + if (has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> /*
> @@ -1385,7 +1379,7 @@ static void stop_passive_scanning_complete(uint8_t status, uint16_t length,
> * no need to stop any discovery. The kernel will handle the
> * auto-connection by itself.
> */
> - if (kernel_conn_control)
> + if (has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> /*
> @@ -2816,7 +2810,7 @@ static void property_set_mode_complete(uint8_t status, uint16_t length,
>
> static void clear_discoverable(struct btd_adapter *adapter)
> {
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> if (!(adapter->current_settings & MGMT_SETTING_DISCOVERABLE))
> @@ -2876,7 +2870,7 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
>
> break;
> case MGMT_SETTING_DISCOVERABLE:
> - if (kernel_conn_control) {
> + if (has_kernel_features(KERNEL_CONN_CONTROL)) {
> if (mode) {
> set_mode(adapter, MGMT_OP_SET_CONNECTABLE,
> mode);
> @@ -4193,7 +4187,8 @@ static void load_default_system_params(struct btd_adapter *adapter)
> size_t len = 0;
> unsigned int err;
>
> - if (!main_opts.default_params.num_entries || !kernel_set_system_config)
> + if (!main_opts.default_params.num_entries ||
> + !has_kernel_features(KERNEL_SET_SYSTEM_CONFIG))
> return;
>
> params = malloc0(sizeof(*params) *
> @@ -4878,7 +4873,7 @@ int adapter_connect_list_add(struct btd_adapter *adapter,
> * adapter_auto_connect_add() function is used to maintain what to
> * connect.
> */
> - if (kernel_conn_control)
> + if (has_kernel_features(KERNEL_CONN_CONTROL))
> return 0;
>
> if (g_slist_find(adapter->connect_list, device)) {
> @@ -4918,7 +4913,7 @@ void adapter_connect_list_remove(struct btd_adapter *adapter,
> if (device == adapter->connect_le)
> adapter->connect_le = NULL;
>
> - if (kernel_conn_control)
> + if (has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> if (!g_slist_find(adapter->connect_list, device)) {
> @@ -4980,7 +4975,7 @@ void adapter_whitelist_add(struct btd_adapter *adapter, struct btd_device *dev)
> {
> struct mgmt_cp_add_device cp;
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> memset(&cp, 0, sizeof(cp));
> @@ -5019,7 +5014,7 @@ void adapter_whitelist_remove(struct btd_adapter *adapter, struct btd_device *de
> {
> struct mgmt_cp_remove_device cp;
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> memset(&cp, 0, sizeof(cp));
> @@ -5075,7 +5070,7 @@ void adapter_auto_connect_add(struct btd_adapter *adapter,
> uint8_t bdaddr_type;
> unsigned int id;
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> if (g_slist_find(adapter->connect_list, device)) {
> @@ -5147,7 +5142,7 @@ void adapter_set_device_wakeable(struct btd_adapter *adapter,
> const bdaddr_t *bdaddr;
> uint8_t bdaddr_type;
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> bdaddr = device_get_address(device);
> @@ -5224,7 +5219,7 @@ void adapter_auto_connect_remove(struct btd_adapter *adapter,
> uint8_t bdaddr_type;
> unsigned int id;
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> if (!g_slist_find(adapter->connect_list, device)) {
> @@ -6764,7 +6759,7 @@ connect_le:
> * If kernel background scan is used then the kernel is
> * responsible for connecting.
> */
> - if (kernel_conn_control)
> + if (has_kernel_features(KERNEL_CONN_CONTROL))
> return;
>
> /*
> @@ -8964,7 +8959,7 @@ static int clear_devices(struct btd_adapter *adapter)
> {
> struct mgmt_cp_remove_device cp;
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> return 0;
>
> memset(&cp, 0, sizeof(cp));
> @@ -9282,7 +9277,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
> (missing_settings & MGMT_SETTING_FAST_CONNECTABLE))
> set_mode(adapter, MGMT_OP_SET_FAST_CONNECTABLE, 0x01);
>
> - if (kernel_exp_features)
> + if (has_kernel_features(KERNEL_EXP_FEATURES))
> read_exp_features(adapter);
>
> err = adapter_register(adapter);
> @@ -9403,7 +9398,8 @@ static void read_info_complete(uint8_t status, uint16_t length,
>
> set_name(adapter, btd_adapter_get_name(adapter));
>
> - if (kernel_blocked_keys_supported && !set_blocked_keys(adapter)) {
> + if (has_kernel_features(KERNEL_BLOCKED_KEYS_SUPPORTED) &&
> + !set_blocked_keys(adapter)) {
> btd_error(adapter->dev_id,
> "Failed to set blocked keys for index %u",
> adapter->dev_id);
> @@ -9414,7 +9410,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
> !(adapter->current_settings & MGMT_SETTING_BONDABLE))
> set_mode(adapter, MGMT_OP_SET_BONDABLE, 0x01);
>
> - if (!kernel_conn_control)
> + if (!has_kernel_features(KERNEL_CONN_CONTROL))
> set_mode(adapter, MGMT_OP_SET_CONNECTABLE, 0x01);
> else if (adapter->current_settings & MGMT_SETTING_CONNECTABLE)
> set_mode(adapter, MGMT_OP_SET_CONNECTABLE, 0x00);
> @@ -9590,19 +9586,19 @@ static void read_commands_complete(uint8_t status, uint16_t length,
> switch (op) {
> case MGMT_OP_ADD_DEVICE:
> DBG("enabling kernel-side connection control");
> - kernel_conn_control = true;
> + kernel_features |= KERNEL_CONN_CONTROL;
> break;
> case MGMT_OP_SET_BLOCKED_KEYS:
> DBG("kernel supports the set_blocked_keys op");
> - kernel_blocked_keys_supported = true;
> + kernel_features |= KERNEL_BLOCKED_KEYS_SUPPORTED;
> break;
> case MGMT_OP_SET_DEF_SYSTEM_CONFIG:
> DBG("kernel supports set system confic");
> - kernel_set_system_config = true;
> + kernel_features |= KERNEL_SET_SYSTEM_CONFIG;
> break;
> case MGMT_OP_READ_EXP_FEATURES_INFO:
> DBG("kernel supports exp features");
> - kernel_exp_features = true;
> + kernel_features |= KERNEL_EXP_FEATURES;
> break;
> default:
> break;
> @@ -9768,3 +9764,8 @@ bool btd_le_connect_before_pairing(void)
>
> return false;
> }
> +
> +bool has_kernel_features(uint32_t features)
> +{
> + return !!(kernel_features & features);
Don't think we need the !! here, the use () should already be enough,
or change it to return (kernel_features & features) ? true : false; if
you think that is more readable.
> +}
> diff --git a/src/adapter.h b/src/adapter.h
> index f8ac20261..b0ed4915f 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -233,3 +233,12 @@ void btd_adapter_for_each_device(struct btd_adapter *adapter,
> void *data);
>
> bool btd_le_connect_before_pairing(void);
> +
> +enum kernel_features {
> + KERNEL_CONN_CONTROL = 1 << 0,
> + KERNEL_BLOCKED_KEYS_SUPPORTED = 1 << 1,
> + KERNEL_SET_SYSTEM_CONFIG = 1 << 2,
> + KERNEL_EXP_FEATURES = 1 << 3,
> +};
> +
> +bool has_kernel_features(uint32_t feature);
> --
> 2.28.0.618.gf4bc123cb7-goog
>
--
Luiz Augusto von Dentz
