Bug 36216 - dbus_g_proxy_add_signal docs make false claims of introspection support
Summary: dbus_g_proxy_add_signal docs make false claims of introspection support
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://cgit.freedesktop.org/~smcv/dbu...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-04-13 11:03 UTC by Simon McVittie
Modified: 2011-05-30 07:37 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

Attachments
dbus_g_proxy_add_signal: stop falsely claiming that we read introspection (1.66 KB, patch)
2011-04-13 11:05 UTC, Simon McVittie 		Details | Splinter Review
dbus_g_proxy_add_signal: document error cases (1.09 KB, patch)
2011-04-13 11:05 UTC, Simon McVittie 		Details | Splinter Review
dbus_g_proxy_add_signal: document error cases (patch v2) (1.03 KB, patch)
2011-04-19 03:04 UTC, Simon McVittie 		Details | Splinter Review
Show Obsolete (2) View All

Description Simon McVittie 2011-04-13 11:03:40 UTC 
18:25 < neal> the service supports introspection
18:25 < smcv> that's not relevant
18:25 < smcv> dbus-glib never uses introspection
...
18:26 < neal> then dbus_g_proxy_add_signal () is required?
18:26 < smcv> oh yeah, there used to be a documentation bug where it claimed it 
              was unnecessary
18:26 < smcv> is that still true? :-(
...
18:27 < smcv> if that documentation was actually true I'd consider it to be a 
              critical bug, since that would let the signal-emitter crash 
              dbus-glib by providing wrong arguments
18:27 < smcv> possibly with arbitrary code execution
18:27 < smcv> we should fix the documentation though
Comment 1 Simon McVittie 2011-04-13 11:05:13 UTC 
Created attachment 45582 [details] [review]
dbus_g_proxy_add_signal: stop falsely claiming that we read introspection

If we believed the introspection, services could change their
introspection and remote-crash us, so it's good that we don't. We
shouldn't claim that we do, though.

The second sentence is subtle: for D-Bus types that dbus-glib can map
into more than one GLib type, you must currently use the one that
dbus-glib would "naturally" produce. The only example I can find is that
object paths must be DBUS_TYPE_G_OBJECT_PATH, even though dbus-glib can
also (in principle) unmarshal object-paths as DBUS_TYPE_G_PROXY.
Comment 2 Simon McVittie 2011-04-13 11:05:30 UTC 
Created attachment 45583 [details] [review]
dbus_g_proxy_add_signal: document error cases
Comment 3 Colin Walters 2011-04-13 11:10:58 UTC 
Review of attachment 45582 [details] [review]:

Looks great.
Comment 4 Simon McVittie 2011-04-14 04:42:54 UTC 
(In reply to comment #3)
> Review of attachment 45582 [details] [review]:
> 
> Looks great.

Fixed in git for 0.93, thanks. How about the other one?
Comment 5 Simon McVittie 2011-04-19 03:04:53 UTC 
Created attachment 45804 [details] [review]
dbus_g_proxy_add_signal: document error cases (patch v2)

Same patch, adjusted to apply now that Bug #23616 has been fixed.
Comment 6 Guillaume Desmottes 2011-05-30 07:35:13 UTC 
Review of attachment 45804 [details] [review]:

Looks good (assuming that's actually an error :)
Comment 7 Simon McVittie 2011-05-30 07:37:13 UTC 
Thanks; fixed in git for 0.94 based on Guillaume's review + no objections from the reviewer group

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.