[pulseaudio-discuss] [PATCH] bluetooth: Unregister endpoints when unloading

[Mikel Astiz]

Hi Arun,

On Thu, Sep 27, 2012 at 6:15 AM, Arun Raghavan
<arun.raghavan at collabora.co.uk> wrote:
> This makes sure that we explicitly unregister any registered endpoints
> when unloading module-bluetooth-*, since the corresponding state on the
> bluetoothd side is only cleaned up when PulseAudio as a whole gets
> killed.

As a side comment, keep in mind that this includes unloading
module-bluetooth-discover. So while I agree with the change, the
impact of this would be rather low since I can't think of any use-case
doing this.

> ---
>  src/modules/bluetooth/bluetooth-util.c |   51 ++++++++++++++++++++++++++++++++
>  1 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index cb7b69d..89b2049 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -62,6 +62,13 @@
>      " </interface>"                                                     \
>      "</node>"
>
> +typedef struct pa_bluetooth_adapter pa_bluetooth_adapter;
> +
> +struct pa_bluetooth_adapter {
> +    char *path;
> +    PA_LLIST_FIELDS(pa_bluetooth_adapter);
> +};
> +
>  struct pa_bluetooth_discovery {
>      PA_REFCNT_DECLARE;
>
> @@ -71,6 +78,7 @@ struct pa_bluetooth_discovery {
>      pa_hashmap *devices;
>      pa_hook hook;
>      pa_bool_t filter_added;
> +    PA_LLIST_HEAD(pa_bluetooth_adapter, adapters);
>  };
>
>  static void get_properties_reply(DBusPendingCall *pending, void *userdata);
> @@ -705,8 +713,51 @@ static void register_endpoint(pa_bluetooth_discovery *y, const char *path, const
>      send_and_add_to_pending(y, m, register_endpoint_reply, pa_xstrdup(endpoint));
>  }
>
> +static void unregister_endpoint_reply(DBusPendingCall *pending, void *userdata) {
> +    DBusError e;
> +    DBusMessage *r;
> +    pa_dbus_pending *p;
> +    pa_bluetooth_discovery *y;
> +
> +    pa_assert(pending);
> +
> +    dbus_error_init(&e);
> +
> +    pa_assert_se(p = userdata);
> +    pa_assert_se(y = p->context_data);
> +    pa_assert_se(r = dbus_pending_call_steal_reply(pending));
> +
> +    if (dbus_message_is_error(r, DBUS_ERROR_SERVICE_UNKNOWN)) {
> +        pa_log_debug("Bluetooth daemon is apparently not available.");
> +        goto finish;
> +    }
> +
> +    if (dbus_message_get_type(r) == DBUS_MESSAGE_TYPE_ERROR) {
> +        pa_log("org.bluez.Media.UnregisterEndpoint() failed: %s: %s", dbus_message_get_error_name(r), pa_dbus_get_error_message(r));
> +        goto finish;

You might want to remove this goto.

> +    }
> +
> +finish:
> +    dbus_message_unref(r);
> +
> +    PA_LLIST_REMOVE(pa_dbus_pending, y->pending, p);
> +    pa_dbus_pending_free(p);
> +}
> +
> +static void unregister_endpoint(pa_bluetooth_discovery *y, const char *path, const char *endpoint) {
> +    DBusMessage *m;
> +
> +    pa_log_debug("Unregistering %s on adapter %s.", endpoint, path);
> +
> +    pa_assert_se(m = dbus_message_new_method_call("org.bluez", path, "org.bluez.Media", "UnregisterEndpoint"));
> +    pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_OBJECT_PATH, &endpoint, DBUS_TYPE_INVALID));
> +
> +    send_and_add_to_pending(y, m, unregister_endpoint_reply, NULL);
> +}
> +
>  static void found_adapter(pa_bluetooth_discovery *y, const char *path) {
>      DBusMessage *m;
> +    pa_bluetooth_adapter *adapter;

This line looks unrelated.

>
>      pa_assert_se(m = dbus_message_new_method_call("org.bluez", path, "org.bluez.Adapter", "GetProperties"));
>      send_and_add_to_pending(y, m, get_properties_reply, NULL);
> --

More interestingly, you are not calling unregister_endpoint anywhere.
I guess the place would be pa_bluetooth_discovery_unref?

Cheers,
Mikel