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) $ 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. -- 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
