Patrick,
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:
ln -s src/backends/webdav/syncevo-webdav-lookup.sh
src/backends/webdav/syncevo-webdav-lookup
/bin/bash ./libtool --tag=CXX --mode=compile g++ -DHAVE_CONFIG_H
-I. -I/home/chaiwala/devel/syncevolution/src -I./src/gdbusxx -I./test
-I/usr/include -pthread -I/usr/include/gnome-keyring-1
-I/usr/include/libxml2 -I/usr/include/libsoup-2.4
-I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include
-I/usr/include/nss -I/usr/include/nspr
-I/usr/include/evolution-data-server-3.6 -pthread
-I/usr/include/gnome-keyring-1 -I/usr/include/libxml2
-I/usr/include/libsoup-2.4 -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/nss
-I/usr/include/nspr -I/usr/include/evolution-data-server-3.6
-pthread -I/usr/include/nss -I/usr/include/nspr
-I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include
-I/usr/include/gnome-keyring-1 -I/usr/include/libxml2
-I/usr/include/libsoup-2.4 -I/usr/include/evolution-data-server-3.6
-I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include
-pthread -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -Wall
-Wno-unknown-pragmas -Wno-deprecated-declarations -pthread
-I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include
-g -O2 -MT src/backends/pbap/src_backends_pbap_syncpbap_la-PbapSyncSource.lo
-MD -MP -MF
src/backends/pbap/.deps/src_backends_pbap_syncpbap_la-PbapSyncSource.Tpo
-c -o src/backends/pbap/src_backends_pbap_syncpbap_la-PbapSyncSource.lo
`test -f 'src/backends/pbap/PbapSyncSource.cpp' || echo
'./'`src/backends/pbap/PbapSyncSource.cpp
libtool: compile: g++ -DHAVE_CONFIG_H -I.
-I/home/chaiwala/devel/syncevolution/src -I./src/gdbusxx -I./test
-I/usr/include -pthread -I/usr/include/gnome-keyring-1
-I/usr/include/libxml2 -I/usr/include/libsoup-2.4
-I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include
-I/usr/include/nss -I/usr/include/nspr
-I/usr/include/evolution-data-server-3.6 -pthread
-I/usr/include/gnome-keyring-1 -I/usr/include/libxml2
-I/usr/include/libsoup-2.4 -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/nss
-I/usr/include/nspr -I/usr/include/evolution-data-server-3.6 -pthread
-I/usr/include/nss -I/usr/include/nspr -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include
-I/usr/include/gnome-keyring-1 -I/usr/include/libxml2
-I/usr/include/libsoup-2.4 -I/usr/include/evolution-data-server-3.6
-I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include
-pthread -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -Wall -Wno-unknown-pragmas
-Wno-deprecated-declarations -pthread -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -g -O2 -MT
src/backends/pbap/src_backends_pbap_syncpbap_la-PbapSyncSource.lo -MD
-MP -MF src/backends/pbap/.deps/src_backends_pbap_syncpbap_la-PbapSyncSource.Tpo
-c src/backends/pbap/PbapSyncSource.cpp -fPIC -DPIC -o
src/backends/pbap/.libs/src_backends_pbap_syncpbap_la-PbapSyncSource.o
In file included from src/backends/pbap/PbapSyncSource.cpp:45:0:
./src/gdbusxx/gdbus-cxx-bridge.h: In static member function 'static
void GDBusCXX::dbus_traits<std::map<K, V, C>
>::append(GVariantBuilder&, GDBusCXX::dbus_traits<std::map<K, V, C>
>::arg_type) [with K = std::basic_string<char>, V =
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>,
C = std::less<std::basic_string<char> >, GVariantBuilder =
_GVariantBuilder, GDBusCXX::dbus_traits<std::map<K, V, C> >::arg_type
= const std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>&, GDBusCXX::dbus_traits<std::map<K, V, C> >::host_type =
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>]':
./src/gdbusxx/gdbus-cxx-bridge.h:339:9: instantiated from
'GDBusCXX::AppendRetvals& GDBusCXX::AppendRetvals::operator<<(const
A&) [with A = std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>, GDBusCXX::AppendRetvals = GDBusCXX::AppendRetvals]'
./src/gdbusxx/gdbus-cxx-bridge.h:4404:9: instantiated from
'GDBusCXX::DBusClientCall<CallTraits>::Return_t
GDBusCXX::DBusClientCall<CallTraits>::operator()(const A1&, const A2&)
[with A1 = std::basic_string<char>, A2 =
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>, CallTraits = GDBusCXX::Ret1Traits<GDBusCXX::DBusObject_t>,
GDBusCXX::DBusClientCall<CallTraits>::Return_t =
GDBusCXX::DBusObject_t]'
src/backends/pbap/PbapSyncSource.cpp:163:48: instantiated from here
./src/gdbusxx/gdbus-cxx-bridge.h:1718:13: error: 'append' is not a
member of 'GDBusCXX::dbus_traits<boost::variant<std::basic_string<char>,
GDBusCXX::DBusObject_t, int> >'
./src/gdbusxx/gdbus-cxx-bridge.h: In static member function 'static
std::string GDBusCXX::dbus_traits<std::map<K, V, C>
>::getContainedType() [with K = std::basic_string<char>, V =
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>,
C = std::less<std::basic_string<char> >, std::string =
std::basic_string<char>]':
./src/gdbusxx/gdbus-cxx-bridge.h:1716:13: instantiated from 'static
void GDBusCXX::dbus_traits<std::map<K, V, C>
>::append(GVariantBuilder&, GDBusCXX::dbus_traits<std::map<K, V, C>
>::arg_type) [with K = std::basic_string<char>, V =
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>,
C = std::less<std::basic_string<char> >, GVariantBuilder =
_GVariantBuilder, GDBusCXX::dbus_traits<std::map<K, V, C> >::arg_type
= const std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>&, GDBusCXX::dbus_traits<std::map<K, V, C> >::host_type =
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>]'
./src/gdbusxx/gdbus-cxx-bridge.h:339:9: instantiated from
'GDBusCXX::AppendRetvals& GDBusCXX::AppendRetvals::operator<<(const
A&) [with A = std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>, GDBusCXX::AppendRetvals = GDBusCXX::AppendRetvals]'
./src/gdbusxx/gdbus-cxx-bridge.h:4404:9: instantiated from
'GDBusCXX::DBusClientCall<CallTraits>::Return_t
GDBusCXX::DBusClientCall<CallTraits>::operator()(const A1&, const A2&)
[with A1 = std::basic_string<char>, A2 =
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>, CallTraits = GDBusCXX::Ret1Traits<GDBusCXX::DBusObject_t>,
GDBusCXX::DBusClientCall<CallTraits>::Return_t =
GDBusCXX::DBusObject_t]'
src/backends/pbap/PbapSyncSource.cpp:163:48: instantiated from here
./src/gdbusxx/gdbus-cxx-bridge.h:1673:13: error: 'getType' is not a
member of 'GDBusCXX::dbus_traits<boost::variant<std::basic_string<char>,
GDBusCXX::DBusObject_t, int> >'
./src/gdbusxx/gdbus-cxx-bridge.h: In static member function 'static
void GDBusCXX::dbus_traits<std::map<K, V, C>
>::get(GDBusCXX::ExtractArgs&, GVariantIter&,
GDBusCXX::dbus_traits<std::map<K, V, C> >::host_type&) [with K =
std::basic_string<char>, V = boost::variant<std::basic_string<char>,
GDBusCXX::DBusObject_t, int>, C = std::less<std::basic_string<char> >,
GVariantIter = _GVariantIter, GDBusCXX::dbus_traits<std::map<K, V, C>
>::host_type = std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>]':
./src/gdbusxx/gdbus-cxx-bridge.h:1580:9: instantiated from 'static
void GDBusCXX::dbus_traits<std::pair<_T1, _T2>
>::get(GDBusCXX::ExtractArgs&, GVariantIter&,
GDBusCXX::dbus_traits<std::pair<_T1, _T2> >::host_type&) [with A =
GDBusCXX::DBusObject_t, B = std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>
>, GVariantIter = _GVariantIter, GDBusCXX::dbus_traits<std::pair<_T1,
_T2> >::host_type = std::pair<GDBusCXX::DBusObject_t,
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
>]'
./src/gdbusxx/gdbus-cxx-bridge.h:467:9: instantiated from
'GDBusCXX::ExtractArgs& GDBusCXX::Get<A>::get(GDBusCXX::ExtractArgs&)
const [with A = std::pair<GDBusCXX::DBusObject_t,
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
>]'
./src/gdbusxx/gdbus-cxx-bridge.h:457:27: instantiated from
'GDBusCXX::ExtractArgs& GDBusCXX::ExtractArgs::operator>>(const A&)
[with A = GDBusCXX::Get<std::pair<GDBusCXX::DBusObject_t,
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
> >, GDBusCXX::ExtractArgs = GDBusCXX::ExtractArgs]'
./src/gdbusxx/gdbus-cxx-bridge.h:4176:9: instantiated from 'static
GDBusCXX::Ret1Traits<R1>::Return_t
GDBusCXX::Ret1Traits<R1>::demarshal(GDBusCXX::DBusMessagePtr&, const
GDBusCXX::DBusConnectionPtr&) [with R1 =
std::pair<GDBusCXX::DBusObject_t, std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
>, GDBusCXX::Ret1Traits<R1>::Return_t =
std::pair<GDBusCXX::DBusObject_t, std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
>]'
./src/gdbusxx/gdbus-cxx-bridge.h:4351:51: instantiated from
'GDBusCXX::DBusClientCall<CallTraits>::Return_t
GDBusCXX::DBusClientCall<CallTraits>::sendAndReturn(GDBusCXX::DBusMessagePtr&)
[with CallTraits =
GDBusCXX::Ret1Traits<std::pair<GDBusCXX::DBusObject_t,
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
> >, GDBusCXX::DBusClientCall<CallTraits>::Return_t =
std::pair<GDBusCXX::DBusObject_t, std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
>]'
./src/gdbusxx/gdbus-cxx-bridge.h:4387:33: instantiated from
'GDBusCXX::DBusClientCall<CallTraits>::Return_t
GDBusCXX::DBusClientCall<CallTraits>::operator()(const A1&) [with A1 =
std::basic_string<char>, CallTraits =
GDBusCXX::Ret1Traits<std::pair<GDBusCXX::DBusObject_t,
std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
> >, GDBusCXX::DBusClientCall<CallTraits>::Return_t =
std::pair<GDBusCXX::DBusObject_t, std::map<std::basic_string<char>,
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int> >
>]'
src/backends/pbap/PbapSyncSource.cpp:256:88: instantiated from here
./src/gdbusxx/gdbus-cxx-bridge.h:1702:13: error: 'get' is not a member
of 'GDBusCXX::dbus_traits<boost::variant<std::basic_string<char>,
GDBusCXX::DBusObject_t, int> >'
In file included from src/backends/pbap/PbapSyncSource.cpp:447:0:
./src/gdbusxx/gdbus-cxx-bridge.h: In static member function 'static
std::string GDBusCXX::dbus_traits<std::map<K, V, C>
>::getContainedType() [with K = std::basic_string<char>, V =
boost::variant<std::basic_string<char>, GDBusCXX::DBusObject_t, int>,
C = std::less<std::basic_string<char> >, std::string =
std::basic_string<char>]':
./src/gdbusxx/gdbus-cxx-bridge.h:1674:5: warning: control reaches end
of non-void function [-Wreturn-type]
make[2]: *** [src/backends/pbap/src_backends_pbap_syncpbap_la-PbapSyncSource.lo]
Error 1
make[2]: Leaving directory `/home/chaiwala/devel/syncevolution'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/chaiwala/devel/syncevolution'
make: *** [all] Error 2
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?
thanks,
Jeremy
On Fri, Sep 21, 2012 at 2:06 PM, Patrick Ohly <patrick.o...@intel.com> wrote:
> Hello Jeremy!
>
> Here my comments for the code that you pushed yesterday.
>
> Your latest code revision still creates string copies instead of just
> splitting up the memory mapped file:
>
> commit dcd892e84a630c394f9f3ac4615bed09d8c6bede
> Author: Jeremy Whiting <jeremy.whit...@collabora.com>
> Date: Fri Aug 31 18:18:01 2012 -0600
>
> PBAP: Fix vcard parsing regexp so all vcards are parsed.
>
> diff --git a/src/backends/pbap/PbapSyncSource.cpp
> b/src/backends/pbap/PbapSyncSource.cpp
> index b854633..d39dbec 100644
> --- a/src/backends/pbap/PbapSyncSource.cpp
> +++ b/src/backends/pbap/PbapSyncSource.cpp
> @@ -280,13 +280,13 @@ void PbapSession::pullAll(Content &dst)
>
> pcrecpp::StringPiece content(addr, sb.st_size);
>
> - string vcarddata;
> + pcrecpp::StringPiece vcarddata;
> int count = 0;
> - pcrecpp::RE re("(^BEGIN:VCARD.*?^END:VCARD)",
> + pcrecpp::RE re("[\\r\\n]*(^BEGIN:VCARD.*?^END:VCARD)",
>
> pcrecpp::RE_Options().set_dotall(true).set_multiline(true));
> while (re.Consume(&content, &vcarddata)) {
> std::string id = StringPrintf("%d", count);
> - dst[id] = vcarddata;
> + dst[id] = vcarddata.as_string();
> ^^^^^^^^^^^^^^^^^^^^^^
>
> I still think it would be better to rearrange the class so that dst maps
> from id to a StringPiece, with the actual memory owned by the class
> (either in the memory mapped area or in a std::string buffer).
>
> The parsing code should be identical for both code paths.
>
> Unmapping the memory mapped file is missing.
>
> The comment for the commit should describe the commit in enough detail
> that a reader of the commit log knows how this new functionality works
> (key points: dynamic runtime check, tries new API first), and what the
> limitations/requirements are (needs D-Bus daemon >= xxx - forgot the
> exact number).
>
> It only works as long as obexd only sends strings as property values.
> This is not guaranteed. As I said earlier, "Instead we need a
> boost::variant<std::string, GDBusCXX::DBusObject_t,
> int, bool, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t,
> uint64_t> as value for the map".
>
> This should be added to the commit messages and as bug entry in the
> issue tracker, otherwise there is a high risk that it'll get into a
> release without fixing it. Heck, I probably should insist on it getting
> fixed before merging into master :-/
>
> I'd prefer to merge one patch for the PBAP backend. Feel free to squash
> your and my commits into one.
>
> --
> 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
SyncEvolution@syncevolution.org
http://lists.syncevolution.org/listinfo/syncevolution
