[pulseaudio-discuss] [PATCH 01/17] bluetooth: Monitor D-Bus signals

[Tanu Kaskinen]

On Fri, 2014-08-22 at 17:04 +0300, Luiz Augusto von Dentz wrote:
> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> 
> ---
>  src/modules/bluetooth/backend-ofono.c | 49 ++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
> index bf0db47..f5fe472 100644
> --- a/src/modules/bluetooth/backend-ofono.c
> +++ b/src/modules/bluetooth/backend-ofono.c
> @@ -30,6 +30,7 @@
>  
>  #define OFONO_SERVICE "org.ofono"
>  #define HF_AUDIO_AGENT_INTERFACE OFONO_SERVICE ".HandsfreeAudioAgent"
> +#define HF_AUDIO_MANAGER_INTERFACE OFONO_SERVICE ".HandsfreeAudioManager"
>  
>  #define HF_AUDIO_AGENT_PATH "/HandsfreeAudioAgent"
>  
> @@ -55,8 +56,16 @@
>  struct pa_bluetooth_backend {
>      pa_core *core;
>      pa_dbus_connection *connection;
> +    pa_hashmap *cards;
>  };
>  
> +static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *data) {
> +    pa_assert(bus);
> +    pa_assert(m);
> +
> +    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> +}
> +
>  static DBusMessage *hf_audio_agent_release(DBusConnection *c, DBusMessage *m, void *data) {
>      DBusMessage *r = dbus_message_new_error(m, "org.ofono.Error.NotImplemented", "Operation is not implemented");
>      return r;
> @@ -115,12 +124,35 @@ pa_bluetooth_backend *pa_bluetooth_backend_new(pa_core *c) {
>  
>      backend = pa_xnew0(pa_bluetooth_backend, 1);
>      backend->core = c;
> +    backend->cards = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>  
>      dbus_error_init(&err);
>  
>      if (!(backend->connection = pa_dbus_bus_get(c, DBUS_BUS_SYSTEM, &err))) {
>          pa_log("Failed to get D-Bus connection: %s", err.message);
>          dbus_error_free(&err);
> +        pa_xfree(backend);
> +        return NULL;
> +    }
> +
> +    /* dynamic detection of handsfree audio cards */
> +    if (!dbus_connection_add_filter(pa_dbus_connection_get(backend->connection), filter_cb, backend, NULL)) {
> +        pa_log_error("Failed to add filter function");
> +        pa_dbus_connection_unref(backend->connection);
> +        pa_xfree(backend);
> +        return NULL;
> +    }
> +
> +    if (pa_dbus_add_matches(pa_dbus_connection_get(backend->connection), &err,
> +            "type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',"
> +            "arg0='" OFONO_SERVICE "'",
> +            "type='signal',sender='" OFONO_SERVICE "',interface='" HF_AUDIO_MANAGER_INTERFACE "',member='CardAdded'",
> +            "type='signal',sender='" OFONO_SERVICE "',interface='" HF_AUDIO_MANAGER_INTERFACE "',member='CardRemoved'",
> +            NULL) < 0) {
> +        pa_log("Failed to add oFono D-Bus matches: %s", err.message);
> +        dbus_connection_remove_filter(pa_dbus_connection_get(backend->connection), filter_cb, backend);
> +        pa_dbus_connection_unref(backend->connection);
> +        pa_xfree(backend);
>          return NULL;
>      }
>  
> @@ -133,11 +165,20 @@ pa_bluetooth_backend *pa_bluetooth_backend_new(pa_core *c) {
>  void pa_bluetooth_backend_free(pa_bluetooth_backend *backend) {
>      pa_assert(backend);
>  
> -    if (backend->connection) {
> -        dbus_connection_unregister_object_path(pa_dbus_connection_get(backend->connection), HF_AUDIO_AGENT_PATH);
> +    dbus_connection_unregister_object_path(pa_dbus_connection_get(backend->connection), HF_AUDIO_AGENT_PATH);
>  
> -        pa_dbus_connection_unref(backend->connection);
> -    }
> +    pa_dbus_remove_matches(pa_dbus_connection_get(backend->connection),
> +            "type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',"
> +            "arg0='" OFONO_SERVICE "'",
> +            "type='signal',sender='" OFONO_SERVICE "',interface='" HF_AUDIO_MANAGER_INTERFACE "',member='CardAdded'",
> +            "type='signal',sender='" OFONO_SERVICE "',interface='" HF_AUDIO_MANAGER_INTERFACE "',member='CardRemoved'",

Maybe it would be a good idea to define a macro for the matches, so that
it would be easier to keep the added and removed matches in sync?

> +            NULL);
> +
> +    dbus_connection_remove_filter(pa_dbus_connection_get(backend->connection), filter_cb, backend);
> +
> +    pa_dbus_connection_unref(backend->connection);
> +
> +    pa_hashmap_free(backend->cards);
>  
>      pa_xfree(backend);
>  }

I can't say I really like the changes that you did to João's patch
(except fixing the bug that was there earlier). Now you assume in free()
that new() has completed all initialization, and the error paths in
new() need to carefully undo all initializations that have been done
earlier in the function. That works with the current code, but it's very
easy to accidentally break it if new() is modified. I'll let you decide
whether to change that, however.

-- 
Tanu