Re: [SyncEvolution] C++ D-Bus bindings + blocking calls
2012/2/16 Patrick Ohly <patrick.ohly(a)intel.com>:
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.
> Also, my cssr1
> branch is based on Chris' concurrent-sync-sessions-r1 branch. I should
> have put this DBus blocking calls work into separate one based on
> master I guess.
I'm working on that now. Basically I am cherry-picking everything
related to src/gdbus or src/gdbusxx from your cssr1 and my
for-master/dbus-blocking. I'm not squashing anything yet, so hopefully
rebasing cssr1 onto it will work automatically.
Shall we preserve the block() functionality? It might be useful in some
cases (avoid extra copying), but unless it really gets used it's just
dead code. Perhaps preserve it on a branch instead or merging into
master?
block() functionality is used in resources in
concurrent-sync-sessions-r1 branch. Same with genericCb() and
bindGeneric() in DBusClientCall{0,1,2,3}. I leave that decision to you
- maybe you will find them useful elsewhere - especially for
synchronous calls when all you want to do with return values is to
assign them to some class members. Like in the following code:
std::string error;
call.block(in1, in2, call.bindGeneric(&m_out1, m_out2, &str_error));
if (!error.empty()) { throw Foo; }
> > I don't have a good solution for string constants while
sticking to ISO
> > C++, that's why they are not used much in the SyncEvolution code base.
>
> This commit should be applied against concurrent-sync-sessions-r1 I
> believe - there we have changed SessionCommon from class into
> namespace [1], so all my string constants work.
Duh, my bad. You are right of course, in a namespace it is fine. I
missed that in your commit, and then my own test only used them in a
class.
--
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.