On Thu, 2012-08-30 at 13:37 -0600, Jeremy Whiting wrote:
> On Thu, Aug 30, 2012 at 12:32 AM, Patrick Ohly <[email protected]> wrote:
> > On Wed, 2012-08-29 at 15:13 -0600, Jeremy Whiting wrote:
> >> Ok, I've updated my syncpbap branch here:
> >> http://cgit.collabora.com/git/user/jwhiting/syncevolution.git but the
> >> completeCb and errorCb both never get called, so it just sits there.
> >> The transfer does happen, as the file is created and vcard data pumped
> >> in,  but something is not right with the signalwatch.
> >
> > Are you now running with a dbus daemon >= 1.5.0? I'm asking because I
> > don't see any changes in your branch to make the path prefix filtering
> > work with the older dbus that you were running.
> 
> Yes, I've upgraded dbus to version 1.6.4 now.

Then let's hope that all users with obexd > 0.47 in their distro also
have a recent enough dbus and not bother about adding the fallback code.

> Ok, I've attached the log here, it looks to me that we are getting
> notified of the Complete (and the Progress signals also) but we don't
> seem to be doing anything with them.  The completeCb is never called,
> as you don't see "obexd transfer" debug messages in the log anywhere.
> Is there a way to check if the boost::bind on those callbacks is
> working?  I don't see any warnings or error messages at runtime
> either, but maybe boost supresses those or something?

It already failed in SignalFilter::matches(). I had a redundant m_path
check in it - sorry for that. I could have sworn that I tested that
version of the code before pushing, but obviously I only tested the
version without that check. 

I also noticed another bug and had to fix a compiler warning in your
code. All changes are in jwhiting-pbap:

commit 0e6428cdae8e5c81d13d8d0b9d66c18921f0301c
Author: Patrick Ohly <[email protected]>
Date:   Fri Aug 31 16:33:36 2012 +0200

    PBAP: fixed compiler warning
    
    %d != long

diff --git a/src/backends/pbap/PbapSyncSource.cpp 
b/src/backends/pbap/PbapSyncSource.cpp
index 61299f4..b854633 100644
--- a/src/backends/pbap/PbapSyncSource.cpp
+++ b/src/backends/pbap/PbapSyncSource.cpp
@@ -270,7 +270,7 @@ void PbapSession::pullAll(Content &dst)
             SE_LOG_ERROR(NULL, NULL, "Stat on file failed");
             return;
         }
-        SE_LOG_DEBUG(NULL, NULL, "Temporary file size is %d", 
(long)sb.st_size);
+        SE_LOG_DEBUG(NULL, NULL, "Temporary file size is %ld", 
(long)sb.st_size);
 
         addr = (char*)mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
         if (addr == MAP_FAILED) {

commit 24f7d177f411ebb23dfdabbc802459db57aef5bf
Author: Patrick Ohly <[email protected]>
Date:   Fri Aug 31 16:31:52 2012 +0200

    GDBus GIO: fix SignalFilter
    
    The signal name was not stored, making the actual filtering more
    relaxed than it should have been. Worse, the path was redundantly
    checked for a complete match even when SIGNAL_FILTER_PATH_PREFIX was
    set, thus preventing signal deliver in that case.

diff --git a/src/gdbusxx/gdbus-cxx-bridge.h b/src/gdbusxx/gdbus-cxx-bridge.h
index 40fba39..05c53c7 100644
--- a/src/gdbusxx/gdbus-cxx-bridge.h
+++ b/src/gdbusxx/gdbus-cxx-bridge.h
@@ -4686,6 +4686,7 @@ class SignalFilter : public DBusRemoteObject
                  const std::string &signal,
                  Flags flags) :
         DBusRemoteObject(conn, path, interface, ""),
+        m_signal(signal),
         m_flags(flags)
     {}
 
@@ -4701,7 +4702,6 @@ class SignalFilter : public DBusRemoteObject
     bool matches(const ExtractArgs &context) const
     {
         return
-            (m_path.empty() || m_path == context.m_path) &&
             (m_interface.empty() || m_interface == context.m_interface) &&
             (m_signal.empty() || m_signal == context.m_signal) &&
             (m_path.empty() ||


> >
> > +        string vcarddata;
> >
> > Should be StringPiece.
> 
> If I change that to pcrecpp::StringPiece vcarddata it no longer
> builds, since Consume expects it's second argument to be a
> std::string.

Huh? The second argument is a result variable for the first bracket.
Passing the address of a StringPiece should be fine. Works fine here
with pcre3 8.30 (Debian).

With the fix for GDBus C++, the --print-items operation completes, but
parsing the result fails. I only get the first contact although the
temporary transfer file had more data. At first glance it looks like
Consume() is unhappy once it has leading \n\r in the content StringPiece
(not matched by the regex anywhere)... yes, that's it.

With this change here it works for me:

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();
 
             ++count;
         }


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