[pulseaudio-discuss] [PATCH 4/5] bluetooth: Remove device_remove_all()

[João Paulo Rechi Vita]

On Sun, Sep 29, 2013 at 12:49 PM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> The function did two things: set device_info_valid to -1 and called
> device_free() for each device in the hashmap. Setting
> device_info_valid to -1 was unnecessary. The main purpose of that was
> to fire DEVICE_CONNECTION_CHANGED as a side effect, but that hook is
> fired anyway in device_free(), as a side effect of removing all
> transports. Calling device_free() can be delegated to pa_hashmap, when
> freeing or emptying it.
> ---
>  src/modules/bluetooth/bluez5-util.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index c7e934e..591ea50 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -454,17 +454,6 @@ static void set_device_info_valid(pa_bluetooth_device *device, int valid) {
>          pa_hook_fire(&device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED], device);
>  }
>
> -static void device_remove_all(pa_bluetooth_discovery *y) {
> -    pa_bluetooth_device *d;
> -
> -    pa_assert(y);
> -
> -    while ((d = pa_hashmap_steal_first(y->devices))) {
> -        set_device_info_valid(d, -1);
> -        device_free(d);
> -   }
> -}
> -
>  static pa_bluetooth_adapter* adapter_create(pa_bluetooth_discovery *y, const char *path) {
>      pa_bluetooth_adapter *a;
>
> @@ -928,7 +917,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
>          if (pa_streq(name, BLUEZ_SERVICE)) {
>              if (old_owner && *old_owner) {
>                  pa_log_debug("Bluetooth daemon disappeared");
> -                device_remove_all(y);
> +                pa_hashmap_remove_all(y->devices);
>                  pa_hashmap_remove_all(y->adapters);
>                  y->objects_listed = false;
>              }
> @@ -1533,7 +1522,8 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c) {
>      y->core = c;
>      y->adapters = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL,
>                                        (pa_free_cb_t) adapter_free);
> -    y->devices = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> +    y->devices = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL,
> +                                     (pa_free_cb_t) device_free);
>      y->transports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>      PA_LLIST_HEAD_INIT(pa_dbus_pending, y->pending);
>
> @@ -1608,10 +1598,8 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
>
>      pa_dbus_free_pending_list(&y->pending);
>
> -    if (y->devices) {
> -        device_remove_all(y);
> +    if (y->devices)
>          pa_hashmap_free(y->devices);
> -    }
>
>      if (y->adapters)
>          pa_hashmap_free(y->adapters);

Ack. Same question about testing applies here, and I should have made
it more generic, actually: Do you have audio devices to be able to
test these changes?

-- 
João Paulo Rechi Vita
http://about.me/jprvita