On Mo, 2011-12-19 at 16:47 +0100, Chris Kühl wrote:
> On Mon, Dec 19, 2011 at 12:08 PM, Patrick Ohly <patrick.o...@intel.com> wrote:
> > So perhaps the parameter for that can be removed and made a static
> > choice in the dbus_get_bus_connection() implementations?
> >
> 
> I'm about half way through test-dbus.py and things are looking fine
> using the singleton connection acquired by g_bus_get_sync(). so, if my
> interpretation of what you're saying is correct, we can just remove
> the unshared (btw, using a negative for a boolean value is confusing.)
> option and make the gio gdbus always be a shared, singleton connection
> and the libdbus connection always be private. Correct?

Right.

> > So let me be more precise: a "syncevolution --version" invocation
> > segfaults.
> >
> 
> Hmm. I get SyncEvolution 1.2.1+20111218+SE+3f4f71a (pre-release) when
> running that on my Debian Testing install. But we have had different
> results in the past on Deban Testing so I'm becoming less and less
> surprised by this.

Well, Testing is a moving target ;-) We might also be compiling
differently, or there is a 32/64 bit difference (I'm using 64 bit).

Anyway, as I am to reproduce it, finding the root cause wasn't that
difficult:

commit b7b17159d9e89c0452e3df99a35668a468e347d7
Author: Patrick Ohly <patrick.o...@intel.com>
Date:   Mon Dec 19 17:57:20 2011 +0100

    GIO GDBus: fixed memory corruption
    
    g_variant_get() does not work for C++ bool directly because at least
    on x86_64, C++ bool uses 1 byte while gboolean uses 4. This caused
    random stack corruption when unpacking four bytes into an address
    which only had room for one. Caused "syncevolution --version" to segfault
    when using a corrupted std::string later.

diff --git a/src/gdbusxx/gdbus-cxx-bridge.h b/src/gdbusxx/gdbus-cxx-bridge.h
index 106b216..568642b 100644
--- a/src/gdbusxx/gdbus-cxx-bridge.h
+++ b/src/gdbusxx/gdbus-cxx-bridge.h
@@ -1170,12 +1170,37 @@ template<> struct dbus_traits<uint32_t> :
     static std::string getReply() { return ""; }
 };
 
-template<> struct dbus_traits<bool> :
-    public basic_marshal< bool, VariantTypeBoolean >
+template<> struct dbus_traits<bool>
+// cannot use basic_marshal because VariantTypeBoolean packs/unpacks
+// a gboolean, which is not a C++ bool (4 bytes vs 1 on x86_64)
+// public basic_marshal< bool, VariantTypeBoolean >
 {
     static std::string getType() { return "b"; }
     static std::string getSignature() {return getType(); }
     static std::string getReply() { return ""; }
+
+    typedef bool host_type;
+    typedef bool arg_type;
+
+    static void get(GDBusConnection *conn, GDBusMessage *msg,
+                    GVariantIter &iter, bool &value)
+    {
+        GVariant *var = g_variant_iter_next_value(&iter);
+        if (var == NULL || !g_variant_type_equal(g_variant_get_type(var), 
VariantTypeBoolean::getVariantType())) {
+            throw std::runtime_error("invalid argument");
+        }
+        gboolean buffer;
+        g_variant_get(var, g_variant_get_type_string(var), &buffer);
+        value = buffer;
+        g_variant_unref(var);
+    }
+
+    static void append(GVariantBuilder &builder, bool value)
+    {
+        const gchar *typeStr = 
g_variant_type_dup_string(VariantTypeBoolean::getVariantType());
+        g_variant_builder_add(&builder, typeStr, (gboolean)value);
+        g_free((gpointer)typeStr);
+    }
 };

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

Reply via email to