This one’s a bit more involved than the other fixes. Introduce a steal() function to make it clear when hash table functions steal their arguments. For all other cases (mostly for handling the `goto out` paths), use a top-level str_key and str_value which are cleared at the end of the function.
Created attachment 129553 [details] [review] dbus-hash: Fix memory leaks in internal hash table tests This includes fixing a memory leak in _dbus_hash_iter_lookup(), which is not one of the unit tests; but it is only ever called from the unit tests, so this is not a user-facing leak. Coverity IDs: 54730, 54740 Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment on attachment 129553 [details] [review] dbus-hash: Fix memory leaks in internal hash table tests Review of attachment 129553 [details] [review]: ----------------------------------------------------------------- Makes sense.
Fixed in git for 1.11.10, thanks
Bisection says this makes tests fail: dbus-daemon[26876]: Activating service name='org.freedesktop.DBus.TestSuiteEchoService' requested by ':1.2415' (uid=1000 pid=26876 comm=".../bus/.libs/test-bus ") dbus-daemon[26876]: Failed to activate service 'org.freedesktop.DBus.TestSuiteEchoService': timed out (service_start_timeout=25000ms) dbus-daemon[26876]: Did not expect error org.freedesktop.DBus.Error.TimedOut I reverted it.
It's possible that this is exposing some other wrongness, maybe an issue in an out-of-memory code path: we're doing this from test-bus and not really dbus-daemon.
Created attachment 129665 [details] [review] dbus-hash: Fix memory leaks in internal hash table tests This includes fixing a memory leak in _dbus_hash_iter_lookup(), which is not one of the unit tests; but it is only ever called from the unit tests, so this is not a user-facing leak. Coverity IDs: 54730, 54740 Signed-off-by: Philip Withnall <withnall@endlessm.com>
The previous version of the patch introduced a double-free in the case where find_generic_function() was called from the more traditional insertion functions (like _dbus_hash_table_insert_string_preallocated()). This version is a bit less invasive, and only modifies _dbus_hash_iter_lookup(), which is only ever called from the tests. The tests pass with this patch applied.
Comment on attachment 129665 [details] [review] dbus-hash: Fix memory leaks in internal hash table tests Review of attachment 129665 [details] [review]: ----------------------------------------------------------------- Looks good.
Fixed in git for 1.11.10, coming soon.
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.