Bug 91008 - Memleak when GetConnectionCredentials is successful
Summary: Memleak when GetConnectionCredentials is successful
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.8
Hardware: All All
: medium major
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
Keywords: patch
Depends on:
Reported: 2015-06-17 11:53 UTC by Jacek Bukarewicz
Modified: 2015-06-18 08:39 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Proposed fix for memleak in GetConnectionCredentials handler (651 bytes, patch)
2015-06-17 11:53 UTC, Jacek Bukarewicz
Details | Splinter Review

Description Jacek Bukarewicz 2015-06-17 11:53:03 UTC
Created attachment 116556 [details] [review]
Proposed fix for memleak in GetConnectionCredentials handler

I've run dbus-daemon in valgrind and it reported memleaks after calling GetConnectionCredentials with valid unique id as the parameter.
The reason for that seems to be lack of proper reply message unreferencing in GetConnectionCredentials handler. Proposed fix in attachment.

I would be grateful to apply this fix also to 1.8.x branch.
Comment 1 Simon McVittie 2015-06-17 18:00:24 UTC
Comment on attachment 116556 [details] [review]
Proposed fix for memleak in GetConnectionCredentials handler

Review of attachment 116556 [details] [review]:

The general idea looks right, the actual change not so much.

Please use git format-patch instead of git show, it makes it much easier to preserve attribution when applying patches.

::: bus/driver.c
@@ +1755,4 @@
>        goto oom;
>      }
> +  bus_message_unref (reply);

Did you test this? There is no function of that name.

There is a dbus_message_unref() whose use here would fix the bug, though :-)
Comment 2 Simon McVittie 2015-06-17 19:44:58 UTC
Fixed in git for 1.8.20 and 1.9.18 (with function name corrected).
Comment 3 Jacek Bukarewicz 2015-06-18 08:39:32 UTC
Yes, you are right. Sorry for being careless. I tested it on Tizen D-Bus repository with dbus_message_unref and it worked correctly. Then I wrote similar patch in the upstream repo making a typo by the way without testing it again.

Thanks for correcting and applying the fix.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.