| Summary: | Add dbus_clear_connection(), etc. macros | ||
|---|---|---|---|
| Product: | dbus | Reporter: | Simon McVittie <smcv> |
| Component: | core | Assignee: | Simon McVittie <smcv> |
| Status: | RESOLVED FIXED | QA Contact: | D-Bus Maintainers <dbus> |
| Severity: | enhancement | ||
| Priority: | medium | Keywords: | patch |
| Version: | git master | ||
| Hardware: | Other | ||
| OS: | All | ||
| URL: | https://github.com/smcv/dbus/commits/dbus-clear-101895 | ||
| Whiteboard: | review+ | ||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 101354 | ||
| Attachments: |
Implement dbus_clear_connection(), etc.
tests: Use dbus_clear_connection etc. in a couple of tests Implement dbus_clear_connection(), etc. tests: Use dbus_clear_connection etc. in a couple of tests |
||
Created attachment 132864 [details] [review] Implement dbus_clear_connection(), etc. These are inspired by GLib's g_clear_pointer() and g_clear_object(), which in turn is descended from CPython's Py_CLEAR_OBJECT. They should make our code a lot less repetitive. Created attachment 132865 [details] [review] tests: Use dbus_clear_connection etc. in a couple of tests This is just enough to demonstrate that they work - I'm deliberately not doing a mass change throughout all tests, and we should definitely not rush to introduce these into production code, because it would hinder cherry-picking and merging fixes between branches. However, new code on master can use them freely. These are deliberately not called things like dbus_message_clear(), for two reasons: * You might think dbus_message_clear() does something else, like maybe removing the body and all the headers * They are not really pseudo-OO instance methods on a DBusMessage etc. - their first argument is a DBusMessage **, not a DBusMessage * as you would expect for a pseudo-OO method Comment on attachment 132864 [details] [review] Implement dbus_clear_connection(), etc. Review of attachment 132864 [details] [review]: ----------------------------------------------------------------- YES. r+++++ Comment on attachment 132865 [details] [review] tests: Use dbus_clear_connection etc. in a couple of tests Review of attachment 132865 [details] [review]: ----------------------------------------------------------------- r+ Created attachment 133104 [details] [review] Implement dbus_clear_connection(), etc. These are inspired by GLib's g_clear_pointer() and g_clear_object(), which in turn is descended from CPython's Py_CLEAR_OBJECT. They should make our code a lot less repetitive. --- Be more careful with types and casts, to keep C++ compilers happy (see comment). This broke the build with mingw on Travis-CI, because we use a tiny amount of C++ on Windows to get code run at library init time in a non-horrific way. Created attachment 133105 [details] [review] tests: Use dbus_clear_connection etc. in a couple of tests This is just enough to demonstrate that they work - I'm deliberately not doing a mass change throughout all tests, and we should definitely not rush to introduce these into production code, because it would hinder cherry-picking and merging fixes between branches. However, new code on master can use them freely. --- I don't think I changed this, but I've lost track. Comment on attachment 133104 [details] [review] Implement dbus_clear_connection(), etc. Review of attachment 133104 [details] [review]: ----------------------------------------------------------------- r+ still Thanks, merged for 1.11.18. |
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.
GLib has g_clear_object() and g_clear_pointer() macros which are really useful: GProxyResolver *object = NULL; /* or any other GObject */ struct { ...; GArray *arr; ...; } *my_struct; ... if (...) { object = ...; my_struct->arr = ...; } out: g_clear_object (&object); g_clear_pointer (&my_struct->arr, g_array_unref); They were inspired by Python's Py_CLEAR_OBJECT() which has the same semantics. They expand to something like this: GArray *tmp = *(&arr); /* or equivalently, arr */ *(&arr) = NULL; if (tmp != NULL) g_array_unref (tmp); (In particular the use of a temporary variable means the struct member is cleared before the user-supplied free-function is called, which is safer if the free-function might cause reentrancy.) We can make libdbus less painful to program in by doing something similar.