There are a couple of errors in dbus_g_proxy_end_call_internal() related to freeing memory after an error condition. First of all the va_list args_unwind is never initialized. I looked back at earlier versions of this file and found that it was initialized when this code was first added, but the initialization seems to have been lost in a refactoring of the code. Second, va_end() is called on args eventhough this is done in all calling functions (where it belongs AFAIK). Third, the clean up code at the end of dbus_g_proxy_end_call_internal() frees the first n_retvals_processed arguments, regardless of the fact that every other argument is the type of the next argument... Fourth, it tries to free all arguments regardless of type. And fifth, it should free what the arguments point at rather than the arguments themselves.
Created attachment 9838 [details] [review] Correct argument unwinding in dbus_g_proxy_end_call_internal() This patch should fix all the problems I have reported. Possibly, the switch statement freeing the arguments depending on their type should be in some helper function in dbus-gvalue-utils.c.
Created attachment 9839 [details] [review] Correct argument unwinding in dbus_g_proxy_end_call_internal() Small update to the patch for better coding style conformance.
I forgot to mention that I have not tried the code for freeing G_TYPE_BOXED and G_TYPE_OBJECT as we do not use either in our code, but I hope I got it right.
Rob, this looks good to me, want to have a look?
This patch had some flaws: o It removed the call to va_end (args) o We want to handle all argument types, not just the few in the switch. So we use the internal _dbus_gvalue_take which uses our GValue machinery to handle argument types in a generic way I fixed those issues and also some others, see the log: commit 43db9baa4cd0921d2ee830185ab46b4646b4e73b Author: Colin Walters <walters@verbum.org> Date: Tue May 27 16:49:26 2008 -0400 Bug 10834: Fix error handling path for dbus_g_proxy_end_call_internal This patch was based initial work by Peter Kjellerstedt. This patch made obvious the need to correctly handle type mismatches in demarshal_basic, similarly to what the other demarshalers are doing. Also add some tests for error handling.
*** Bug 9063 has been marked as a duplicate of this bug. ***
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.