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.ohly(a)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.whiting(a)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.