12:15 a.m.
2012/2/16 Patrick Ohly <patrick.ohly(a)intel.com>:
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.
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. :)