On Thu, 2012-09-27 at 09:39 +0200, Mikel Astiz wrote:
> 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 dwhole 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.
Thanks for the review. The patch was incomplete as you might have
guessed, so complete patch coming up after this.
As for the use case, it's a simple one - you want to use PulseAudio for
all your audio except Bluetooth (i.e. something else is registering an
endpoint and handling all that).
> > ---
> > src/modules/bluetooth/bluetooth-util.c | 51 ++++++++++++++++++++++++++++++++
> > 1 files changed, 51 insertions(+), 0 deletions(-)
[...]
> > +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.
I usually keep it there so it's harder to miss adding that if some code
is added after the condition but before finish.
Cheers,
Arun
