Re: [SyncEvolution] C++ D-Bus bindings + blocking calls
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 <patrick.ohly(a)intel.com>:
> > 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-syncevolut...
> >
> > 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 <krnowak(a)openismus.com>
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.