ayatana-commits team mailing list archive

[Mikkel Kamstrup Erlandsen <mikkel.kamstrup@xxxxxxxxx>]

Review: Approve
I haven't looked deeply into this, but it looks very nice at glance. Very good work Ted! :-)

Because I on principle *must* comment something on such large reviews here's some feedback. All of these are really just personal programming style preferences, so disregard to your liking:

 a) I think that all the heap allocated GVariantBuilders and GVariantIters could just as well be stack allocated. Not that the increased memchurn by heap allocations will matter much compared to the socket io.

 b) For methods that return guint32 and gint32 i prefer also storing the return values not just in a guint or gint, since technically guint and int could be 64 bit. Although it's extremely unlikely to cause problems, it's theoretically possible (I think?)

 c) In your async calls where you pass 'self' as the userdata argument I think you need to g_object_ref(self) and the unref in the async callback. The self instance may be finalized the async call returns. This may not be a problem in this particular lib, but I've had problems with that in some of my own libs in the past.
-- 
https://code.launchpad.net/~ted/dbusmenu/kill-dbus-glib/+merge/42543
Your team ayatana-commits is subscribed to branch lp:dbusmenu.