Re: [pulseaudio-discuss] [PATCH] fix dbus message leaks
On Sat, 2017-07-15 at 11:08 +0530, Arun Raghavan wrote:
>
> On Mon, 3 Jul 2017, at 08:05 PM, Tanu Kaskinen wrote:
> > I reviewed all places that call
> > dbus_connection_send_with_reply_and_block(), and found several places
> > where dbus messages aren't properly unreffed.
> > ---
> > src/modules/bluetooth/backend-ofono.c | 6 ++
> > src/modules/bluetooth/bluez4-util.c | 12 ++--
> > src/modules/bluetooth/bluez5-util.c | 12 ++--
> > src/modules/reserve.c | 6 ++
> > 4 files changed, 32 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/modules/bluetooth/backend-ofono.c
> > b/src/modules/bluetooth/backend-ofono.c
> > index 6e9a3664e..2c51497f3 100644
> > --- a/src/modules/bluetooth/backend-ofono.c
> > +++ b/src/modules/bluetooth/backend-ofono.c
> > @@ -168,9 +168,15 @@ static int
> > hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
> > dbus_error_init();
> > pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> > "org.ofono.HandsfreeAudioCard", "Connect"));
> > r =
> >
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
> > m, -1, );
> > +dbus_message_unref(m);
> > +m = NULL;
> > +
> > if (!r)
> > return -1;
> >
> > +dbus_message_unref(r);
> > +r = NULL;
> > +
> > if (card->connecting)
> > return -EAGAIN;
> > }
> > diff --git a/src/modules/bluetooth/bluez4-util.c
> > b/src/modules/bluetooth/bluez4-util.c
> > index 82654508f..ca606193d 100644
> > --- a/src/modules/bluetooth/bluez4-util.c
> > +++ b/src/modules/bluetooth/bluez4-util.c
> > @@ -1116,6 +1116,8 @@ int pa_bluez4_transport_acquire(pa_bluez4_transport
> > *t, bool optional, size_t *i
> > pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> > "org.bluez.MediaTransport", "Acquire"));
> > pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING,
> > , DBUS_TYPE_INVALID));
> > r =
> >
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> > m, -1, );
> > +dbus_message_unref(m);
> > +m = NULL;
> >
> > if (!r) {
> > dbus_error_free();
> > @@ -1143,7 +1145,7 @@ fail:
> >
> > void pa_bluez4_transport_release(pa_bluez4_transport *t) {
> > const char *accesstype = "rw";
> > -DBusMessage *m;
> > +DBusMessage *m, *r;
> > DBusError err;
> >
> > pa_assert(t);
> > @@ -1154,7 +1156,13 @@ void
> > pa_bluez4_transport_release(pa_bluez4_transport *t) {
> >
> > pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> > "org.bluez.MediaTransport", "Release"));
> > pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING,
> > , DBUS_TYPE_INVALID));
> > -
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> > m, -1, );
> > +r =
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> > m, -1, );
> > +dbus_message_unref(m);
> > +m = NULL;
> > +if (r) {
> > +dbus_message_unref(r);
> > +r = NULL;
> > +}
> >
> > if (dbus_error_is_set()) {
> > pa_log("Failed to release transport %s: %s", t->path,
> > err.message);
> > diff --git a/src/modules/bluetooth/bluez5-util.c
> > b/src/modules/bluetooth/bluez5-util.c
> > index 8956fb13a..c92832329 100644
> > --- a/src/modules/bluetooth/bluez5-util.c
> > +++ b/src/modules/bluetooth/bluez5-util.c
> > @@ -363,6 +363,8 @@ static int
> > bluez5_transport_acquire_cb(pa_bluetooth_transport *t, bool optional,
> > dbus_error_init();
> >
> > r =
> >
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> > m, -1, );
> > +dbus_message_unref(m);
> > +m = NULL;
> > if (!r) {
> > if (optional && pa_streq(err.name,
> > "org.bluez.Error.NotAvailable"))
> > pa_log_info("Failed optional acquire of unavailable
> > transport %s", t->path);
> > @@ -393,7 +395,7 @@ finish:
> > }
> >
> > static void bluez5_transport_release_cb(pa_bluetooth_transport *t) {
> > -DBusMessage *m;
> > +DBusMessage *m, *r;
> > DBusError err;
> >
> > pa_assert(t);
> > @@ -408,7 +410,13 @@ static void
> > bluez5_transport_release_cb(pa_bluetooth_transport *t) {
> > }
> >
> > pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> > BLUEZ_MEDIA_TRANSPORT_INTERFACE, "Release"));
> > -
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> > m, -1, );
> > +r =
> > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> > m, -1, );
> > +dbus_message_unref(m);
> > +m = NULL;
> > +if
Re: [pulseaudio-discuss] [PATCH] fix dbus message leaks
On Mon, 3 Jul 2017, at 08:05 PM, Tanu Kaskinen wrote:
> I reviewed all places that call
> dbus_connection_send_with_reply_and_block(), and found several places
> where dbus messages aren't properly unreffed.
> ---
> src/modules/bluetooth/backend-ofono.c | 6 ++
> src/modules/bluetooth/bluez4-util.c | 12 ++--
> src/modules/bluetooth/bluez5-util.c | 12 ++--
> src/modules/reserve.c | 6 ++
> 4 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/src/modules/bluetooth/backend-ofono.c
> b/src/modules/bluetooth/backend-ofono.c
> index 6e9a3664e..2c51497f3 100644
> --- a/src/modules/bluetooth/backend-ofono.c
> +++ b/src/modules/bluetooth/backend-ofono.c
> @@ -168,9 +168,15 @@ static int
> hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
> dbus_error_init();
> pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> "org.ofono.HandsfreeAudioCard", "Connect"));
> r =
>
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
> m, -1, );
> +dbus_message_unref(m);
> +m = NULL;
> +
> if (!r)
> return -1;
>
> +dbus_message_unref(r);
> +r = NULL;
> +
> if (card->connecting)
> return -EAGAIN;
> }
> diff --git a/src/modules/bluetooth/bluez4-util.c
> b/src/modules/bluetooth/bluez4-util.c
> index 82654508f..ca606193d 100644
> --- a/src/modules/bluetooth/bluez4-util.c
> +++ b/src/modules/bluetooth/bluez4-util.c
> @@ -1116,6 +1116,8 @@ int pa_bluez4_transport_acquire(pa_bluez4_transport
> *t, bool optional, size_t *i
> pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> "org.bluez.MediaTransport", "Acquire"));
> pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING,
> , DBUS_TYPE_INVALID));
> r =
>
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> m, -1, );
> +dbus_message_unref(m);
> +m = NULL;
>
> if (!r) {
> dbus_error_free();
> @@ -1143,7 +1145,7 @@ fail:
>
> void pa_bluez4_transport_release(pa_bluez4_transport *t) {
> const char *accesstype = "rw";
> -DBusMessage *m;
> +DBusMessage *m, *r;
> DBusError err;
>
> pa_assert(t);
> @@ -1154,7 +1156,13 @@ void
> pa_bluez4_transport_release(pa_bluez4_transport *t) {
>
> pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> "org.bluez.MediaTransport", "Release"));
> pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING,
> , DBUS_TYPE_INVALID));
> -
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> m, -1, );
> +r =
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> m, -1, );
> +dbus_message_unref(m);
> +m = NULL;
> +if (r) {
> +dbus_message_unref(r);
> +r = NULL;
> +}
>
> if (dbus_error_is_set()) {
> pa_log("Failed to release transport %s: %s", t->path,
> err.message);
> diff --git a/src/modules/bluetooth/bluez5-util.c
> b/src/modules/bluetooth/bluez5-util.c
> index 8956fb13a..c92832329 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -363,6 +363,8 @@ static int
> bluez5_transport_acquire_cb(pa_bluetooth_transport *t, bool optional,
> dbus_error_init();
>
> r =
>
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> m, -1, );
> +dbus_message_unref(m);
> +m = NULL;
> if (!r) {
> if (optional && pa_streq(err.name,
> "org.bluez.Error.NotAvailable"))
> pa_log_info("Failed optional acquire of unavailable
> transport %s", t->path);
> @@ -393,7 +395,7 @@ finish:
> }
>
> static void bluez5_transport_release_cb(pa_bluetooth_transport *t) {
> -DBusMessage *m;
> +DBusMessage *m, *r;
> DBusError err;
>
> pa_assert(t);
> @@ -408,7 +410,13 @@ static void
> bluez5_transport_release_cb(pa_bluetooth_transport *t) {
> }
>
> pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
> BLUEZ_MEDIA_TRANSPORT_INTERFACE, "Release"));
> -
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> m, -1, );
> +r =
> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
> m, -1, );
> +dbus_message_unref(m);
> +m = NULL;
> +if (r) {
> +dbus_message_unref(r);
> +r = NULL;
> +}
>
> if (dbus_error_is_set()) {
> pa_log_error("Failed to release transport %s: %s", t->path,
> err.message);
> diff --git a/src/modules/reserve.c b/src/modules/reserve.c
> index f78805ed7..b0038e662 100644
> ---
[pulseaudio-discuss] [PATCH] fix dbus message leaks
I reviewed all places that call
dbus_connection_send_with_reply_and_block(), and found several places
where dbus messages aren't properly unreffed.
---
src/modules/bluetooth/backend-ofono.c | 6 ++
src/modules/bluetooth/bluez4-util.c | 12 ++--
src/modules/bluetooth/bluez5-util.c | 12 ++--
src/modules/reserve.c | 6 ++
4 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/src/modules/bluetooth/backend-ofono.c
b/src/modules/bluetooth/backend-ofono.c
index 6e9a3664e..2c51497f3 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -168,9 +168,15 @@ static int
hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
dbus_error_init();
pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
"org.ofono.HandsfreeAudioCard", "Connect"));
r =
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
m, -1, );
+dbus_message_unref(m);
+m = NULL;
+
if (!r)
return -1;
+dbus_message_unref(r);
+r = NULL;
+
if (card->connecting)
return -EAGAIN;
}
diff --git a/src/modules/bluetooth/bluez4-util.c
b/src/modules/bluetooth/bluez4-util.c
index 82654508f..ca606193d 100644
--- a/src/modules/bluetooth/bluez4-util.c
+++ b/src/modules/bluetooth/bluez4-util.c
@@ -1116,6 +1116,8 @@ int pa_bluez4_transport_acquire(pa_bluez4_transport *t,
bool optional, size_t *i
pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
"org.bluez.MediaTransport", "Acquire"));
pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING, ,
DBUS_TYPE_INVALID));
r =
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
m, -1, );
+dbus_message_unref(m);
+m = NULL;
if (!r) {
dbus_error_free();
@@ -1143,7 +1145,7 @@ fail:
void pa_bluez4_transport_release(pa_bluez4_transport *t) {
const char *accesstype = "rw";
-DBusMessage *m;
+DBusMessage *m, *r;
DBusError err;
pa_assert(t);
@@ -1154,7 +1156,13 @@ void pa_bluez4_transport_release(pa_bluez4_transport *t)
{
pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
"org.bluez.MediaTransport", "Release"));
pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING, ,
DBUS_TYPE_INVALID));
-
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
m, -1, );
+r =
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
m, -1, );
+dbus_message_unref(m);
+m = NULL;
+if (r) {
+dbus_message_unref(r);
+r = NULL;
+}
if (dbus_error_is_set()) {
pa_log("Failed to release transport %s: %s", t->path, err.message);
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 8956fb13a..c92832329 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -363,6 +363,8 @@ static int
bluez5_transport_acquire_cb(pa_bluetooth_transport *t, bool optional,
dbus_error_init();
r =
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
m, -1, );
+dbus_message_unref(m);
+m = NULL;
if (!r) {
if (optional && pa_streq(err.name, "org.bluez.Error.NotAvailable"))
pa_log_info("Failed optional acquire of unavailable transport %s",
t->path);
@@ -393,7 +395,7 @@ finish:
}
static void bluez5_transport_release_cb(pa_bluetooth_transport *t) {
-DBusMessage *m;
+DBusMessage *m, *r;
DBusError err;
pa_assert(t);
@@ -408,7 +410,13 @@ static void
bluez5_transport_release_cb(pa_bluetooth_transport *t) {
}
pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
BLUEZ_MEDIA_TRANSPORT_INTERFACE, "Release"));
-
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
m, -1, );
+r =
dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection),
m, -1, );
+dbus_message_unref(m);
+m = NULL;
+if (r) {
+dbus_message_unref(r);
+r = NULL;
+}
if (dbus_error_is_set()) {
pa_log_error("Failed to release transport %s: %s", t->path,
err.message);
diff --git a/src/modules/reserve.c b/src/modules/reserve.c
index f78805ed7..b0038e662 100644
--- a/src/modules/reserve.c
+++ b/src/modules/reserve.c
@@ -474,6 +474,9 @@ int rd_acquire(
goto fail;
}
+dbus_message_unref(m);
+m = NULL;
+
if (!dbus_message_get_args(
reply,
error,
@@ -483,6 +486,9 @@ int rd_acquire(
goto fail;
}
+dbus_message_unref(reply);
+reply = NULL;
+
if (!good) {
