Bug 64214 - [PATCH] Use g_cclosure_marshal_generic() when GLib version is sufficient
Summary: [PATCH] Use g_cclosure_marshal_generic() when GLib version is sufficient
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-04 15:12 UTC by Colin Walters
Modified: 2015-02-09 16:29 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-Use-g_cclosure_marshal_generic-when-GLib-version-is-.patch (2.19 KB, patch)
2013-05-04 15:12 UTC, Colin Walters
Details | Splinter Review
0001-Use-g_cclosure_marshal_generic-when-GLib-version-is-.patch (2.29 KB, patch)
2013-05-04 15:14 UTC, Colin Walters
Details | Splinter Review
Require a modern libdbus and a modern GLib (2.94 KB, patch)
2014-09-16 12:37 UTC, Simon McVittie
Details | Splinter Review
Use g_cclosure_marshal_generic for all marshalling (6.70 KB, patch)
2014-09-16 12:38 UTC, Simon McVittie
Details | Splinter Review

Description Colin Walters 2013-05-04 15:12:02 UTC
For applications that use GLib 2.30 or later in combination with
dbus-glib (such as NetworkManager) to simply stop calling
dbus_g_object_register_marshaller().
---
 dbus/dbus-gobject.c   |    7 ++++++-
 test/core/my-object.c |    5 +++++
 2 files changed, 11 insertions(+), 1 deletions(-)
Comment 1 Colin Walters 2013-05-04 15:12:36 UTC
Created attachment 78846 [details] [review]
0001-Use-g_cclosure_marshal_generic-when-GLib-version-is-.patch
Comment 2 Colin Walters 2013-05-04 15:14:38 UTC
Created attachment 78847 [details] [review]
0001-Use-g_cclosure_marshal_generic-when-GLib-version-is-.patch

Improved commit message.
Comment 3 Simon McVittie 2013-08-27 11:21:03 UTC
(In reply to comment #0)
> For applications that use GLib 2.30 or later in combination with
> dbus-glib (such as NetworkManager) to simply stop calling
> dbus_g_object_register_marshaller().

I'd rather not apply this unless we hard-depend on GLib 2.30, and then the application (NetworkManager) hard-depends on the version in which we removed this requirement. Relying on NM and dbus-glib having been compiled against the same GLib seems unwise.

(Consider: distro/sysadmin has GLib 2.28 and compiles dbus-glib against it, then upgrades GLib to 2.30+, then adds NM.)

I'd be OK with adding that hard dependency, though.
Comment 4 Simon McVittie 2014-09-16 12:37:54 UTC
Created attachment 106367 [details] [review]
Require a modern libdbus and a modern GLib

This means we can assume that GLib and libdbus are thread-safe by
default.

Also explicitly document that the object-mapping layer of dbus-glib
is not thread-safe.
Comment 5 Simon McVittie 2014-09-16 12:38:12 UTC
Created attachment 106368 [details] [review]
Use g_cclosure_marshal_generic for all marshalling
Comment 6 Colin Walters 2014-09-16 13:01:43 UTC
Comment on attachment 106368 [details] [review]
Use g_cclosure_marshal_generic for all marshalling

Review of attachment 106368 [details] [review]:
-----------------------------------------------------------------

Hooray for lots of code deletion.  Looks fine to me.
Comment 7 Colin Walters 2014-09-16 13:04:23 UTC
Comment on attachment 106367 [details] [review]
Require a modern libdbus and a modern GLib

Review of attachment 106367 [details] [review]:
-----------------------------------------------------------------

Minor: Depending on both gobject-2.0 and gio-2.0 is pointless as the latter requires the former.

Also, can't we just stop linking to gthread, or does that come later?

::: configure.ac
@@ +263,5 @@
>  AC_SUBST([DBUS_CFLAGS])
>  AC_SUBST([DBUS_LIBS])
>  
>  # Glib detection
> +PKG_CHECK_MODULES([DBUS_GLIB], [gobject-2.0 >= 2.32, gio-2.0 >= 2.32, gthread-2.0 >= 2.32])

If you're touching this, we should also start making use of the new versioning macros.  In other words, also do the equivalent of:

https://git.gnome.org/browse/ostree/tree/Makefile.am?id=dfeb27eca55d923c57735e491e438ae54f8cc201#n25
Comment 8 Simon McVittie 2015-02-09 16:29:46 UTC
(In reply to Colin Walters from comment #7)
> Minor: Depending on both gobject-2.0 and gio-2.0 is pointless as the latter
> requires the former.

Strictly speaking we ought to request gobject-2.0 because we call its functions, although I know GLib has an unconventional pkg-config setup where the libraries guarantee that they are each other's public dependencies.

> Also, can't we just stop linking to gthread, or does that come later?

Not until we stop calling g_thread_init(), which I have now done.

> If you're touching this, we should also start making use of the new
> versioning macros.

Done. We get a lot of deprecation warnings, some of which could be fixed if anyone cared, some of which can't (because GValue is part of our API).

Fixed in 0.104.


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.