| Summary: | tp_presence_mixin_emit_presence_update() segfaults if TP_TYPE_SVC_CONNECTION_INTERFACE_PRESENCE is not implemented | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | butch howard <ext-butch.howard> |
| Component: | tp-glib | Assignee: | Simon McVittie <smcv> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | Keywords: | patch |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/presence-mixin | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
My position on the deprecation is that CMs should still implement complex Presence (since the presence mixin makes it just as easy to do so), but clients are allowed to assume that only SimplePresence matters (telepathy-qt4 and telepathy-glib high-level APIs ignore complex Presence entirely). It's certainly a bug that the presence mixin crashes "late", though; it should either crash out with an assertion failure immediately (in class_init), or cope gracefully. I'll add a check similar to the one you propose in a future version of telepathy-glib. However, I suggest you work around this by adding the necessary 1 line of code to implement old-style Presence too! I can understand that position on the depreciation in this case. From someone new to this set of libraries, 'deprecated' was read in the usual sense of 'not needed at all' or 'to be avoided like the plague' or 'might go away in the near future' (even though there is an existing implementation available). The difference between the CMs and the clients should really be made clear in the places where the complex Presence is marked as deprecated to avoid this confusion for others. To address the immediate need, I have made that one-line change to take advantage of the implementation in the mixin (and thanks for doing that work for us!). Graceful failures with clear indicators of why are much better than abrupt with hard to determine reasons. (In reply to comment #0) > Since INTERFACE_PRESENCE is deprecated, it implies that it is not necessary to > implement that interface. Revisiting this bug, I think it's time we made this true, so I've applied your patch - thanks! I've also rewritten the TpPresenceMixin documentation to make it describe all the necessary actions to wire it up to the Connection. Fixed in 0.11.13 |
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.
Since INTERFACE_PRESENCE is deprecated, it implies that it is not necessary to implement that interface. However, if it is not implemented (even though the TpPresenceMixin does all the work for us it is not clear that it should be used), then a call to tp_presence_mixin_emit_presence_update() will cause a segfault when the internals of tp_svc_connection_interface_presence_emit_presence_update() type check the GOBJECT for the requested interface. A simple fix for this is to wrap the call in a test similar to that done a few lines below for the call for the Simple Presence emit so that the function is not called unless the Presence interface is implemented. Updated Code: void tp_presence_mixin_emit_presence_update (GObject *obj, GHashTable *contact_statuses) { TpPresenceMixinClass *mixin_cls = TP_PRESENCE_MIXIN_CLASS (G_OBJECT_GET_CLASS (obj)); GHashTable *presence_hash; DEBUG ("called."); if (g_type_interface_peek (G_OBJECT_GET_CLASS (obj), TP_TYPE_SVC_CONNECTION_INTERFACE_PRESENCE) != NULL) { presence_hash = construct_presence_hash (mixin_cls->statuses, contact_statuses); tp_svc_connection_interface_presence_emit_presence_update (obj, presence_hash); g_hash_table_destroy (presence_hash); } if (g_type_interface_peek (G_OBJECT_GET_CLASS (obj), TP_TYPE_SVC_CONNECTION_INTERFACE_SIMPLE_PRESENCE) != NULL) { presence_hash = construct_simple_presence_hash (mixin_cls->statuses, contact_statuses); tp_svc_connection_interface_simple_presence_emit_presences_changed (obj, presence_hash); g_hash_table_destroy (presence_hash); } }