2012/2/16 Patrick Ohly <[email protected]>: > On Thu, 2012-02-16 at 10:50 +0100, Patrick Ohly wrote: >> On Thu, 2012-02-16 at 10:03 +0100, Krzesimir Nowak wrote: >> > 2012/2/16 Patrick Ohly <[email protected]>: >> > > My goal is to merge all of the D-Bus binding improvements into master as >> > > soon as possible. I've already rewritten some parts of the syncevolution >> > > binary's D-Bus calls to be blocking and it turned out much nicer than >> > > the older code. The PBAP backend would also benefit from proper blocking >> > > calls. >> > > >> > >> [1] >> > >> https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolution/commits/cssr1 >> > > >> > > Some quick comments about that branch: please write a bit more verbose >> > > commit messages. Just a single line is often not enough to understand >> > > what the commit is about. Looking at the code helps, but takes a lot >> > > more time. If the code is wrong, that kind of analysis doesn't help >> > > either. >> > > >> [...] >> > Ok, I will try to follow the guidelines in my commits. >> >> Thanks, that'll make my life easier. > > For example, why did you change this: > > commit c7a6f28162b5af2d42b41393dc12a16583482312 > Author: Krzesimir Nowak <[email protected]> > Date: Mon Feb 6 16:48:47 2012 +0100 > > GDBus GIO: don't expose GIO GDBus API in wrapper > > In this case - GDBusConnection. Replaced it with DBusConnectionPtr. > > diff --git a/src/gdbusxx/gdbus-cxx-bridge.h > b/src/gdbusxx/gdbus-cxx-bridge.h > index 4d60da0..2464b0b 100644 > --- a/src/gdbusxx/gdbus-cxx-bridge.h > +++ b/src/gdbusxx/gdbus-cxx-bridge.h > @@ -445,7 +445,7 @@ class DBusObject > } > } > > - GDBusConnection *getConnection() const { return m_conn.get(); } > + DBusConnectionPtr getConnection() const { return m_conn; } > .... > > Regardless whether we return GDBusConnection or a DBusConnectionPtr, the > fact that there is a GDBusConnection is visible to the user of the API. > > On the practical side, now with DBusConnectionPtr, a lot of code has to > make explicit get() calls, which I consider a drawback of that approach. > > I stumbled over this because the patch itself is also incomplete. There > are other places where getConnection() still returns a plain pointer. > That in fact breaks compilation of the cssr1 branch in combination with > GDBus libdbus - see my latest push to that branch. > > I'm currently leaning towards simply reverting the commit above. Before > I do that I'd like to understand the motivation for it better. > > -- > Best Regards, Patrick Ohly > > The content of this message is my personal opinion only and although > I am an employee of Intel, the statements I make here in no way > represent Intel's position on the issue, nor am I authorized to speak > on behalf of Intel on this matter. > >
If I remember correctly then we have dbus_get_connection_fd() function that gets a connection pointer. In bluez wrapper connection pointer is marked as DBusConnection and in GIO GDBus wrapper it is GDBusConnection. But C++ intrusive pointer for them is DBusConnectionPtr, so I decided then to change getConnection() methods to return C++ pointer to have single name. I suppose I needed that when I was updating GIO GDBus wrapper to bluez wrapper state. I guess we need neither dbus_get_connection_fd() nor getConnection() returning C++ pointer, so if I were you then I would just get rid of this commit. Sorry for being vague, but my memory on it already blurred. :) _______________________________________________ SyncEvolution mailing list [email protected] http://lists.syncevolution.org/listinfo/syncevolution
