On Tue, 2012-09-25 at 10:40 -0600, Jeremy Whiting wrote:
> Ok, I've added some changes to my branch and will push shortly.  One
> remaining issue is the boost::variant for Params.  If I add int, or
> any of the other plain data types I get errors like this:
[...]

gdbus-cxx-bridge.h must be extended to support boost::variant with more
than two possible types.

However, while looking at that code I found:

/**
 * A boost::variant <V> maps to a dbus variant, only care about values of
 * type V but will not throw error if type is not matched, this is useful if
 * application is interested on only a sub set of possible value types
 * in variant.
 */
...
        if (!boost::iequals(g_variant_get_type_string(varVar), 
dbus_traits<V>::getSignature())) {
            //ignore unrecognized sub type in variant
            return;
        }

In other words, we can keep the PBAP backend code as it is: we will only
store the properties that have strings as values, but as we don't need
anything else (or even the strings) at all, that's okay.

I also found a leak when that return is triggered, but that's a
different story. Will fix it.

> One other issue is that you mentioned I need to unmap the file
> somehow, when should that happen? I would think only on destruction
> since we are using StringPiece's from it until that point, right?

Right. I went ahead and cleaned up the backend, both in the new and in
older code (commit messages below).

The revised code is in the for-master/pbap branch. I'm currently running
a test against that code. Jeremy, shall we merge into master now? I
don't have no objections anymore.

commit 84698cbfed1ae2a53afface7d8f9a832eb3b4780
Author: Patrick Ohly <[email protected]>
Date:   Wed Sep 26 11:51:01 2012 +0200

    GDBus GIO: use RAII for GVariant
    
    Instead of using explicit g_variant_unref() calls, let a smart pointer
    class own the instance. This allows to remove code and (more
    important) fixes leaks when something throws an exception or the
    D-Bus traits for boost::variant return prematurely.

commit d8dce7acc485388caa0eff080c136da20dccd240
Author: Patrick Ohly <[email protected]>
Date:   Wed Sep 26 10:52:33 2012 +0200

    PBAP: clean up and bug fixes for new API support
    
    Fallback to old obexd API was broken because creating the
    DBusRemoteObject does not verify whether the service really exists and
    thus always succeeds. Fixed by checking for existence as part of the
    actual CreateSession method call.
    
    The new code needs glib. Include header file and declare dependency in
    configure.
    
    The backend must throw errors when something fatal happens. Logging
    the error is not enough, because that can't be checked by the
    caller. Throwing errors is best done via the utility methods in
    SyncSource, because those include the source name in the exception.
    
    Memory handling was broken: nothing owned the memory in the
    StringPiece instances, munmap() was missing. Fixed by making
    PbabSyncSource the owner of both.
    
    Unified the parsing of the result. The new code based on pcrecpp is
    used for both old and new obexd API.
    
    File name and GError allocated by g_file_open_tmp() were leaked. The
    file descriptor and the file were leaked in case of aborting via an
    exception. Now these resources are owned by a class which will always
    clean up when getting destructed.
    
    A failed transfer was not checked for when using the new API. Probably
    failed when trying to use the file (because obexd deletes it), but
    better show the error message that we got for the failed transfer.
    
    Remove the obsolete vcardParse().

-- 
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

Reply via email to