[Bug 28782] Migrate from dbus-glib to glib's GDBus

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28782

--- Comment #23 from Xavier Claessens <xclaesse at gmail.com> ---
 - You can remove tp_proxy_get_interface_by_id() completely, it's not used any
more. It creates the proxy with sync call so we shouldn't use it in the future
neither but rather use gdbus-codegen to create GDBusProxy subclasses.

 - tp_svc_interface_set_dbus_interface_info(): doc says there is a vtable arg,
but there isn't.

 - "tp_dbus_daemon_register_object: rewrite in terms of
TpSvcInterfaceSkeleton":
   1) I think GSList's only purpose is making your app crash if you use
g_list_free on it. I see no other reason anyone will ever use it. We are not
going to export millions of objects, so a GList is perfectly good.
   2) I would add a comment in tp_dbus_daemon_unregister_object() telling that
setting the qdata to NULL will call tp_dbus_daemon_registration_free() that
unexport stuff.
   3) g_object_replace_qdata() seems complex, had to read that if condition 3
times. I would just have classic r=g_object_get_qdata(); if(r!=NULL){...;
return;} g_object_set_qdata(); We are not threadsafe so atomic operation isn't
needed and I don't think the perf gain it worth it.

 - "tp_dbus_daemon_dup: rewrite in terms of GDBus, and use session bus":
dbus_g_bus_get() was doing blocking operation as well? Probably not a big deal,
but I'm wondering what would be our API if we want to be fully async.

 - "tp_dbus_daemon_watch_name_owner: rewrite in terms of
g_bus_watch_name_on_conn...": I think it does have a behaviour difference:
g_dbus_watch_name() will call vanished or appeared for the initial status, not
only when it changes later. Also you add in the doc "New code should use
g_bus_watch_name() or similar instead." that sounds like deprecated function,
do you consider 1.0-blocker to actually remove that function?

 - "tp_dbus_daemon_request_name: rewrite using GDBus": Hm, it still uses
dbus-glib's enum for the reply. Maybe we want to write our own if GDBus does
not have helper for that. Maybe open a glib bug to add the helper?

 - "tp_dbus_daemon_release_name: rewrite using GDBus": same.

 - "tp_run_connection_manager: use GDBus to watch for disconnection": commit
msg has a FIXME. You didn't add the g_dbus_connection_set_exit_on_close() and I
think it's fine for the CM-side.

 - "TpProxy: rewrite async and reentrant calls": We still have reentrant calls
in 1.0? I would consider blocker to remove them, or hand-write the few that we
really want and drop the codegen for them, using g_dbus_connection_call_sync()
probably?


I think that's all. I reviewed more the generated code than the generator tbh.
The branch is huge and complex, so I probably overlooked a few stuff, but we
can fix stuff after the merge anyway :)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.