[pulseaudio-discuss] [RFCv0 14/21] bluetooth: Track oFono service

[Tanu Kaskinen]

On Tue, 2014-02-04 at 19:03 -0300, jprvita at gmail.com wrote:
> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> 
> ---
>  src/modules/bluetooth/hfaudioagent-ofono.c | 41 ++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/src/modules/bluetooth/hfaudioagent-ofono.c b/src/modules/bluetooth/hfaudioagent-ofono.c
> index c710caf..23b10f0 100644
> --- a/src/modules/bluetooth/hfaudioagent-ofono.c
> +++ b/src/modules/bluetooth/hfaudioagent-ofono.c
> @@ -379,6 +379,7 @@ static void hf_audio_agent_unregister(hf_audio_agent_data *hfdata) {
>  
>  static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *data) {
>      const char *sender;
> +    DBusError err;
>      hf_audio_agent_data *hfdata = data;
>  
>      pa_assert(bus);
> @@ -389,6 +390,46 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *da
>      if (!pa_safe_streq(hfdata->ofono_bus_id, sender) && !pa_streq("org.freedesktop.DBus", sender))
>          return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>  
> +    dbus_error_init(&err);
> +
> +    if (dbus_message_is_signal(m, "org.freedesktop.DBus", "NameOwnerChanged")) {
> +        const char *name, *old_owner, *new_owner;
> +
> +        if (!dbus_message_get_args(m, &err,
> +                                   DBUS_TYPE_STRING, &name,
> +                                   DBUS_TYPE_STRING, &old_owner,
> +                                   DBUS_TYPE_STRING, &new_owner,
> +                                   DBUS_TYPE_INVALID)) {
> +            pa_log_error("Failed to parse org.freedesktop.DBus.NameOwnerChanged: %s", err.message);
> +            goto fail;
> +        }
> +
> +        if (pa_streq(name, OFONO_SERVICE)) {
> +
> +            if (old_owner && *old_owner) {
> +                pa_log_debug("oFono disappeared");
> +
> +                if (hfdata->hf_audio_cards) {
> +                    pa_hashmap_free(hfdata->hf_audio_cards);
> +                    hfdata->hf_audio_cards = NULL;

It's better to use pa_hashmap_remove_all() rather than to free the whole
hashmap. It should be possible to assume that the hf_audio_cards hashmap
is always non-NULL. (I remember some bug in Tizen that was caused by
this code while other code was assuming that hf_audio_cards was
non-NULL.)

> +                }
> +
> +                if(hfdata->ofono_bus_id) {
> +                    pa_xfree(hfdata->ofono_bus_id);
> +                    hfdata->ofono_bus_id = NULL;
> +                }

No need to use if, pa_xfree() can be called unconditionally.

-- 
Tanu