2012/2/16 Patrick Ohly <[email protected]>: > On Wed, 2012-02-15 at 19:03 +0100, Krzesimir Nowak wrote: >> 2012/2/15 Patrick Ohly <[email protected]>: >> > On Wed, 2012-02-15 at 12:39 +0100, Krzesimir Nowak wrote: >> >> 2012/2/14 Patrick Ohly <[email protected]>: >> >> > We could use an empty tuple to handle a void return type. Same for a >> >> > single return value. But the result wouldn't be very natural: the empty >> >> > tuple could be ignored by the caller, but the single-value very much >> >> > should be the return value, not some tuple containing it. >> >> > >> >> >> This is an idea, so I have no idea if it will work. :) Especially when >> >> >> Return_t is void. >> >> > >> >> > Exactly :-( >> >> > >> >> >> >> Got different idea about this yesterday at home - maybe we could just >> >> use traits class for DBusClientCall. Traits struct would describe >> >> callback type and return type. For void it would some empty struct >> >> like struct VoidReturn {}; >> > >> > That should work. It's a pity that the "void" type is treated >> > differently than other types. >> > >> > Anyway, I also went ahead and implemented the brute-force approach of >> > duplicating different operators this morning. See >> > for-master/dbus-blocking. We can replace that with a more elegant >> > implementation later. >> >> If you are curious then my implementation is in my branch [1] (the >> ada8263c3f20a772928889503b92c272535d8ea8 commit). It should be API >> compatible with your for-master/dbus-blocking code. > > Thanks, that looks nicer. I'll try to use it. But of course you are > cheating with that fake void return code ;-) > > 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. > > From the HACKING document (not all of that applies to your commits; I'm > repeating it here in its entirety because some of the guidelines were > not followed in the D-Bus server rewrite): > > Here are some guidelines for well-formed commits: > - Avoid unnecessary white space changes in the source code. > - Don't mix unrelated changes in one single commit. Every > single commit should leave the code in a functional state > (compiles, unit tests pass, ...). > - Document new functions and structures with Doxygen comments. > These comments should really add information, not just > repeat the name of the entity. At least paraphrase the names. > - Each commit message should follow the format > <item>: <summary> [(BMC #1234)] > <blank line> > <body> > - <item> here refers to the module, class or functionality modified > in the commit, for example "autotools" for the build scripts, > "EvolutionContactSource" for the EDS contact backend, etc. > - <summary> should be consise enough to describe the patch on > its own. > - The optional bugs.meego.com (BMC) number in round brackets > refers to the issue addressed by the commit. Most changes, > in particular bug fixes, should have such an issue filed > for tracking purposes. > - The body (hard-wrapped, multiple paragraphs for long text) > should include the following pieces of information, > if applicable: > - problem statement (*why* is the change relevant?) > - summary of the solution (*how* is the problem solved?) > - what other solutions were considered and rejected (*context* > for the solution) > - known gaps (in particularly useful during code review, > which will start with the oldest, possibly incomplete patch first)
Ok, I will try to follow the guidelines in my commits. 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. > $ git log -p -n 1 2be5a73838a39d36fabf728cff3f16611476eb09 > commit 2be5a73838a39d36fabf728cff3f16611476eb09 > Author: Krzesimir Nowak <[email protected]> > Date: Wed Feb 8 13:31:19 2012 +0100 > > Put some string constants into variables. > ... > diff --git a/src/dbus/server/session-common.h > b/src/dbus/server/session-common.h > index a414047..9f45401 100644 > --- a/src/dbus/server/session-common.h > +++ b/src/dbus/server/session-common.h > @@ -78,7 +78,15 @@ namespace SessionCommon > * never runs operations directly after starting, that shouldn't > * be a problem. > */ > - static const int SHUTDOWN_QUIESCENCE_SECONDS = 10; > + const int SHUTDOWN_QUIESCENCE_SECONDS = 10; > + > + const char * const SERVICE_NAME = "org.syncevolution"; > + const char * const CONNECTION_PATH = > "/org/syncevolution/Connection"; > + const char * const CONNECTION_IFACE = > "org.syncevolution.Connection"; > + const char * const SESSION_PATH = "/org/syncevolution/Session"; > + const char * const SESSION_IFACE = "org.syncevolution.Session"; > + const char * const SERVER_PATH = "/org/syncevolution/Server"; > + const char * const SERVER_IFACE = "org.syncevolution.Server"; > } > > I really wish that this was possible, but unfortunately only C++11 > allows that. clang 3.0-5 compiles it with a warning: > > /tmp/test.h:3:28: warning: in-class initialization of non-static data member > accepted as a C++11 extension > [-Wc++11-extensions] > const char * const FOO = "abc"; > > g++ 4.6.2 fails with an error: > > $ g++ /tmp/test.h -c -o /tmp/test.o > > /tmp/test.h:3:30: error: ISO C++ forbids initialization of member ‘FOO’ > [-fpermissive] > /tmp/test.h:3:30: error: making ‘FOO’ static [-fpermissive] > /tmp/test.h:3:30: error: invalid in-class initialization of static data > member of non-integral type ‘const char* const’ > > 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. [1] https://meego.gitorious.org/meego-middleware/syncevolution/commit/7d861f5a18c1d52e2e97fa4209229b35db217bd8 > -- > 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. > > _______________________________________________ SyncEvolution mailing list [email protected] http://lists.syncevolution.org/listinfo/syncevolution
