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(&derr); > 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, &derr); > + 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, > &accesstype, DBUS_TYPE_INVALID)); > r = > > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), > m, -1, &err); > + dbus_message_unref(m); > + m = NULL; > > if (!r) { > dbus_error_free(&err); > @@ -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, > &accesstype, DBUS_TYPE_INVALID)); > - > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), > m, -1, &err); > + r = > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), > m, -1, &err); > + dbus_message_unref(m); > + m = NULL; > + if (r) { > + dbus_message_unref(r); > + r = NULL; > + } > > if (dbus_error_is_set(&err)) { > 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(&err); > > r = > > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), > m, -1, &err); > + 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, &err); > + r = > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), > m, -1, &err); > + dbus_message_unref(m); > + m = NULL; > + if (r) { > + dbus_message_unref(r); > + r = NULL; > + } > > if (dbus_error_is_set(&err)) { > 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; > +
Why do you reset all these to NULL here when m/r are never referenced again? No harm of course, but it seems verbose, and isn't a pattern we use anywhere else. -- Arun _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss