Re: [PATCH] client: Fix the selection bug of ad manager

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Yunhan,

On Sat, Aug 12, 2017 at 12:13 AM, Yunhan Wang <yunhanw@xxxxxxxxxx> wrote:
> Hi, Luiz
>
> This is the patch regarding advertisement manager selection bug when
> there existing multiple adapters. I have sued git format-patch and git
> send-email. Please help to have a review.
>
> Thanks
> Best wishes
> Yunhan
>
> On Fri, Aug 11, 2017 at 2:09 PM, Yunhan Wang <yunhanw@xxxxxxxxxx> wrote:
>> If there are multiple adapters, bluetoothctl may choose the wrong
>> advertising manager. In addition, select command cannot update
>> current advertising manager when choosing another adapter. Therefore, we
>> need to introduce default_ad_manager to handle the above situation,
>> which is similar to default_ctrl.
>> ---
>>  client/main.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 78 insertions(+), 10 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c
>> index 17de7f81f..1af6f576f 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -65,11 +65,18 @@ struct adapter {
>>         GList *devices;
>>  };
>>
>> +struct LEAdvertisingManager {
>> +       GDBusProxy *proxy;
>> +       char * address;
>> +};

Add the proxy into adapter struct, that way you can access the
ad_manager directly with default_ctrl and we don't need to maintain 2
lists.

>>  static struct adapter *default_ctrl;
>> +static struct LEAdvertisingManager *default_ad_manager;
>>  static GDBusProxy *default_dev;
>>  static GDBusProxy *default_attr;
>> -static GDBusProxy *ad_manager;
>> +static GList *ad_manager_list;
>>  static GList *ctrl_list;
>> +static char *adapter_address;
>>
>>  static guint input = 0;
>>
>> @@ -522,7 +529,7 @@ static void device_added(GDBusProxy *proxy)
>>  static void adapter_added(GDBusProxy *proxy)
>>  {
>>         struct adapter *adapter = g_malloc0(sizeof(struct adapter));
>> -
>> +       DBusMessageIter iter;
>>         adapter->proxy = proxy;
>>         ctrl_list = g_list_append(ctrl_list, adapter);
>>
>> @@ -530,6 +537,22 @@ static void adapter_added(GDBusProxy *proxy)
>>                 default_ctrl = adapter;
>>
>>         print_adapter(proxy, COLORED_NEW);
>> +
>> +       if (g_dbus_proxy_get_property(adapter->proxy, "Address", &iter) == FALSE)
>> +               return;
>> +
>> +       dbus_message_iter_get_basic(&iter, &adapter_address);
>> +}
>> +
>> +static void ad_manager_added(GDBusProxy *proxy)
>> +{
>> +       struct LEAdvertisingManager *ad_manager = g_malloc0(sizeof(struct LEAdvertisingManager));
>> +       ad_manager->proxy = proxy;
>> +       ad_manager->address = adapter_address;
>> +       ad_manager_list = g_list_append(ad_manager_list, ad_manager);
>> +
>> +       if(!default_ad_manager)
>> +               default_ad_manager = ad_manager;
>>  }
>>
>>  static void proxy_added(GDBusProxy *proxy, void *user_data)
>> @@ -560,7 +583,7 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
>>         } else if (!strcmp(interface, "org.bluez.GattManager1")) {
>>                 gatt_add_manager(proxy);
>>         } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
>> -               ad_manager = proxy;
>> +               ad_manager_added(proxy);
>>         }
>>  }
>>
>> @@ -615,6 +638,26 @@ static void adapter_removed(GDBusProxy *proxy)
>>         }
>>  }
>>
>> +static void ad_manager_removed(GDBusProxy *proxy)
>> +{
>> +       GList *ll;
>> +
>> +       for (ll = g_list_first(ctrl_list); ll; ll = g_list_next(ll)) {
>> +               struct LEAdvertisingManager *ad_manager = ll->data;
>> +
>> +               if (ad_manager->proxy == proxy) {
>> +                       if (default_ad_manager && default_ad_manager->proxy == proxy) {
>> +                               default_ad_manager = NULL;
>> +                       }
>> +
>> +                       ad_manager_list = g_list_remove_link(ad_manager_list, ll);
>> +                       g_free(ad_manager);
>> +                       g_list_free(ll);
>> +                       return;
>> +               }
>> +       }
>> +}
>> +
>>  static void proxy_removed(GDBusProxy *proxy, void *user_data)
>>  {
>>         const char *interface;
>> @@ -649,11 +692,9 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
>>         } else if (!strcmp(interface, "org.bluez.GattManager1")) {
>>                 gatt_remove_manager(proxy);
>>         } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
>> -               if (ad_manager == proxy) {
>> -                       agent_manager = NULL;
>> -                       ad_unregister(dbus_conn, NULL);
>> +               ad_unregister(dbus_conn, proxy);
>> +               ad_manager_removed(proxy);
>>                 }
>> -       }
>>  }
>>
>>  static struct adapter *find_ctrl(GList *source, const char *path)
>> @@ -786,6 +827,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
>>         return NULL;
>>  }
>>
>> +static struct LEAdvertisingManager *find_ad_manager_by_address(GList *source, const char *address)
>> +{
>> +       GList *list;
>> +
>> +       for (list = g_list_first(source); list; list = g_list_next(list)) {
>> +               struct LEAdvertisingManager *ad_manager = list->data;
>> +
>> +               if (!strcasecmp(ad_manager->address, address))
>> +                       return ad_manager;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>>  static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
>>  {
>>         GList *list;
>> @@ -922,6 +977,7 @@ static void cmd_show(const char *arg)
>>  static void cmd_select(const char *arg)
>>  {
>>         struct adapter *adapter;
>> +       struct LEAdvertisingManager *ad_manager;
>>
>>         if (!arg || !strlen(arg)) {
>>                 rl_printf("Missing controller address argument\n");
>> @@ -929,6 +985,7 @@ static void cmd_select(const char *arg)
>>         }
>>
>>         adapter = find_ctrl_by_address(ctrl_list, arg);
>> +
>>         if (!adapter) {
>>                 rl_printf("Controller %s not available\n", arg);
>>                 return;
>> @@ -937,7 +994,18 @@ static void cmd_select(const char *arg)
>>         if (default_ctrl && default_ctrl->proxy == adapter->proxy)
>>                 return;
>>
>> +       ad_manager = find_ad_manager_by_address(ad_manager_list, arg);
>> +
>> +       if (!ad_manager) {
>> +               rl_printf("Advertising manager %s not available\n", arg);
>> +               return;
>> +       }
>> +
>> +       if (default_ad_manager && default_ad_manager->proxy == ad_manager->proxy)
>> +               return;
>> +
>>         default_ctrl = adapter;
>> +       default_ad_manager = ad_manager;
>>         print_adapter(adapter->proxy, NULL);
>>  }
>>
>> @@ -2296,15 +2364,15 @@ static void cmd_advertise(const char *arg)
>>         if (parse_argument_advertise(arg, &enable, &type) == FALSE)
>>                 return;
>>
>> -       if (!ad_manager) {
>> +       if (!default_ad_manager || !default_ad_manager->proxy) {
>>                 rl_printf("LEAdvertisingManager not found\n");
>>                 return;
>>         }
>>
>>         if (enable == TRUE)
>> -               ad_register(dbus_conn, ad_manager, type);
>> +               ad_register(dbus_conn, default_ad_manager->proxy, type);
>>         else
>> -               ad_unregister(dbus_conn, ad_manager);
>> +               ad_unregister(dbus_conn, default_ad_manager->proxy);
>>  }
>>
>>  static char *ad_generator(const char *text, int state)
>> --
>> 2.14.0.434.g98096fd7a8-goog
>>



-- 
Luiz Augusto von Dentz
--
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



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux