Attachment 105906 Details for Bug 83396 – refactoring

[patch] refactoring

0001-GVariant-move-common-code-into-libsyncevolution.patch (text/plain), 13.17 KB, created by Patrick Ohly on 2014-09-08 18:46:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Patrick Ohly

Created: 2014-09-08 18:46:21 UTC

Size: 13.17 KB

patch

obsolete

>From a09fd780ebf0ce65a79aa8ccc459ff3eecc41cf8 Mon Sep 17 00:00:00 2001
>From: Patrick Ohly <patrick.ohly@intel.com>
>Date: Mon, 8 Sep 2014 11:34:11 +0200
>Subject: [PATCH 01/10] GVariant: move common code into libsyncevolution
>
>The same code will also be needed by the upcoming oauth2 backend.  The
>header file depends on glib, so don't install it as a public header
>file of libsyncevolution (i.e. only allow using it internally).
>
>The original signon code used a custom GVariant auto_ptr because it
>needed the simple assignment semantic of "transfers ownership". Now
>that the GVariantCXX class is in a shared header file, better make it
>identical to the other CXX classes (i.e., a shared pointer). Avoid
>returning plain pointers because ownership of those cannot be checked
>by the compiler.
>
>The signon case can be handled with GVariantStealCXX, which by default
>takes ownership in the constructor and the default assignment
>operator.
>---
> src/backends/signon/signon.cpp  | 95 ++++++-----------------------------------
> src/dbus/server/pim/folks.h     |  1 -
> src/syncevo/GLibSupport.h       |  1 +
> src/syncevo/GVariantSupport.cpp | 73 +++++++++++++++++++++++++++++++
> src/syncevo/GVariantSupport.h   | 53 +++++++++++++++++++++++
> src/syncevo/syncevo.am          |  2 +
> 6 files changed, 142 insertions(+), 83 deletions(-)
> create mode 100644 src/syncevo/GVariantSupport.cpp
> create mode 100644 src/syncevo/GVariantSupport.h
>
>diff --git a/src/backends/signon/signon.cpp b/src/backends/signon/signon.cpp
>index 3b6e80d..2fb1880 100644
>--- a/src/backends/signon/signon.cpp
>+++ b/src/backends/signon/signon.cpp
>@@ -38,6 +38,7 @@
> #endif
> 
> #include <syncevo/GLibSupport.h>
>+#include <syncevo/GVariantSupport.h>
> #include <pcrecpp.h>
> 
> #include <boost/lambda/core.hpp>
>@@ -54,8 +55,6 @@ SE_GLIB_TYPE(AgService, ag_service)
> SE_GLIB_TYPE(AgAuthData, ag_auth_data)
> #endif
> 
>-SE_GLIB_TYPE(GHashTable, g_hash_table)
>-
> #endif // USE_SIGNON
> 
> #include <syncevo/declarations.h>
>@@ -67,78 +66,6 @@ SE_BEGIN_CXX
> typedef GListCXX<AgService, GList, ag_service_unref> ServiceListCXX;
> #endif
> 
>-/**
>- * Simple auto_ptr for GVariant.
>- */
>-class GVariantCXX : boost::noncopyable
>-{
>-    GVariant *m_var;
>- public:
>-    /** takes over ownership */
>-    GVariantCXX(GVariant *var = NULL) : m_var(var) {}
>-    ~GVariantCXX() { if (m_var) { g_variant_unref(m_var); } }
>-
>-    operator GVariant * () { return m_var; }
>-    GVariantCXX &operator = (GVariant *var) {
>-        if (m_var != var) {
>-            if (m_var) {
>-                g_variant_unref(m_var);
>-            }
>-            m_var = var;
>-        }
>-        return *this;
>-    }
>-};
>-
>-
>-// Originally from google-oauth2-example.c, which is also under LGPL
>-// 2.1, or any later version.
>-static GVariant *HashTable2Variant(const GHashTable *hashTable) throw ()
>-{
>-    GVariantBuilder builder;
>-    GHashTableIter iter;
>-    const gchar *key;
>-    GVariant *value;
>-
>-    if (!hashTable) {
>-        return NULL;
>-    }
>-
>-    g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
>-
>-    g_hash_table_iter_init(&iter, (GHashTable *)hashTable);
>-    while (g_hash_table_iter_next(&iter, (gpointer *)&key, (gpointer *)&value)) {
>-        g_variant_builder_add(&builder, "{sv}", key, g_variant_ref(value));
>-    }
>-    return g_variant_builder_end(&builder);
>-}
>-
>-/**
>- * The created GHashTable maps strings to GVariants which are
>- * reference counted, so when adding or setting values, use g_variant_ref_sink(g_variant_new_...()).
>- */
>-static GHashTable *Variant2HashTable(GVariant *variant) throw ()
>-{
>-    if (!variant) {
>-        return NULL;
>-    }
>-
>-    GHashTable *hashTable;
>-    GVariantIter iter;
>-    GVariant *value;
>-    gchar *key;
>-
>-    hashTable = g_hash_table_new_full(g_str_hash,
>-                                      g_str_equal,
>-                                      g_free,
>-                                      (GDestroyNotify)g_variant_unref);
>-    g_variant_iter_init(&iter, variant);
>-    while (g_variant_iter_next(&iter, "{sv}", &key, &value)) {
>-        g_hash_table_insert(hashTable, g_strdup(key), g_variant_ref(value));
>-    }
>-    return hashTable;
>-}
>-
> class SignonAuthProvider : public AuthProvider
> {
>     SignonAuthSessionCXX m_authSession;
>@@ -165,10 +92,13 @@ public:
>         // Retry login if even the refreshed token failed.
>         g_hash_table_insert(m_sessionData, g_strdup("UiPolicy"),
>                             g_variant_ref_sink(g_variant_new_uint32(failedTokens >= 2 ? SIGNON_POLICY_REQUEST_PASSWORD : 0)));
>-        GVariantCXX resultDataVar;
>+        // We get assigned a plain pointer to an instance that we'll own,
>+        // so we have to use the "steal" variant to enable that assignment.
>+        GVariantStealCXX resultDataVar;
>         GErrorCXX gerror;
>         // Enforce normal reference counting via _ref_sink.
>-        GVariantCXX sessionDataVar(g_variant_ref_sink(HashTable2Variant(m_sessionData)));
>+        GVariantCXX sessionDataVar(g_variant_ref_sink(HashTable2Variant(m_sessionData)),
>+                                   TRANSFER_REF);
>         PlainGStr buffer(g_variant_print(sessionDataVar, true));
>         SE_LOG_DEBUG(NULL, "asking for OAuth2 token with method %s, mechanism %s and parameters %s",
>                      signon_auth_session_get_method(m_authSession),
>@@ -187,7 +117,7 @@ public:
>                                       StringPrintf("could not obtain OAuth2 token: %s", gerror ? gerror->message : "???"),
>                                       STATUS_FORBIDDEN);
>         }
>-        GHashTableCXX resultData(Variant2HashTable(resultDataVar), TRANSFER_REF);
>+        GHashTableCXX resultData(Variant2HashTable(resultDataVar));
>         GVariant *tokenVar = static_cast<GVariant *>(g_hash_table_lookup(resultData, (gpointer)"AccessToken"));
>         if (!tokenVar) {
>             SE_THROW("no AccessToken in OAuth2 response");
>@@ -286,8 +216,8 @@ boost::shared_ptr<AuthProvider> createSignonAuthProvider(const InitStateString &
>     const char *method = ag_auth_data_get_method(authData);
>     const char *mechanism = ag_auth_data_get_mechanism(authData);
> 
>-    GVariantCXX sessionDataVar(ag_auth_data_get_login_parameters(authData, NULL));
>-    GHashTableCXX sessionData(Variant2HashTable(sessionDataVar), TRANSFER_REF);
>+    GVariantCXX sessionDataVar(ag_auth_data_get_login_parameters(authData, NULL), TRANSFER_REF);
>+    GHashTableCXX sessionData(Variant2HashTable(sessionDataVar));
> #ifdef USE_GSSO
>     GVariant *realmsVariant = (GVariant *)g_hash_table_lookup(sessionData, "Realms");
>     PlainGStrArray realms(g_variant_dup_strv(realmsVariant, NULL));
>@@ -360,11 +290,12 @@ boost::shared_ptr<AuthProvider> createSignonAuthProvider(const InitStateString &
> 
>     // 'username' is the part after signon: which we can parse directly.
>     GErrorCXX gerror;
>-    GVariantCXX parametersVar(g_variant_parse(hashtype.get(), username.c_str(), NULL, NULL, gerror));
>+    GVariantCXX parametersVar(g_variant_parse(hashtype.get(), username.c_str(), NULL, NULL, gerror),
>+                              TRANSFER_REF);
>     if (!parametersVar) {
>         gerror.throwError(SE_HERE, "parsing 'signon:' username");
>     }
>-    GHashTableCXX parameters(Variant2HashTable(parametersVar), TRANSFER_REF);
>+    GHashTableCXX parameters(Variant2HashTable(parametersVar));
> 
>     // Extract the values that we expect in the parameters hash.
>     guint32 signonID;
>@@ -398,7 +329,7 @@ boost::shared_ptr<AuthProvider> createSignonAuthProvider(const InitStateString &
>         !g_variant_type_equal(hashtype.get(), g_variant_get_type(value))) {
>         SE_THROW("need 'session: <hash>' in 'signon:' parameters");
>     }
>-    GHashTableCXX sessionData(Variant2HashTable(value), TRANSFER_REF);
>+    GHashTableCXX sessionData(Variant2HashTable(value));
> 
>     SE_LOG_DEBUG(NULL, "using identity %u, method %s, mechanism %s",
>                  signonID, method, mechanism);
>diff --git a/src/dbus/server/pim/folks.h b/src/dbus/server/pim/folks.h
>index 0f6d86f..bf2f9dc 100644
>--- a/src/dbus/server/pim/folks.h
>+++ b/src/dbus/server/pim/folks.h
>@@ -57,7 +57,6 @@ SE_GOBJECT_TYPE(FolksPostalAddressFieldDetails)
> SE_GOBJECT_TYPE(FolksPersona)
> SE_GOBJECT_TYPE(FolksLocation)
> SE_GOBJECT_TYPE(GeeHashSet)
>-SE_GLIB_TYPE(GHashTable, g_hash_table)
> 
> #include <syncevo/declarations.h>
> SE_BEGIN_CXX
>diff --git a/src/syncevo/GLibSupport.h b/src/syncevo/GLibSupport.h
>index 3b16bee..a313026 100644
>--- a/src/syncevo/GLibSupport.h
>+++ b/src/syncevo/GLibSupport.h
>@@ -316,6 +316,7 @@ SE_GOBJECT_TYPE(GFile)
> SE_GOBJECT_TYPE(GFileMonitor)
> SE_GLIB_TYPE(GMainLoop, g_main_loop)
> SE_GLIB_TYPE(GAsyncQueue, g_async_queue)
>+SE_GLIB_TYPE(GHashTable, g_hash_table)
> 
> SE_BEGIN_CXX
> 
>diff --git a/src/syncevo/GVariantSupport.cpp b/src/syncevo/GVariantSupport.cpp
>new file mode 100644
>index 0000000..f131493
>--- /dev/null
>+++ b/src/syncevo/GVariantSupport.cpp
>@@ -0,0 +1,73 @@
>+/*
>+ * Copyright (C) 2014 Intel Corporation
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) version 3.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>+ * 02110-1301  USA
>+ */
>+
>+#include <syncevo/GVariantSupport.h>
>+
>+#include <syncevo/declarations.h>
>+SE_BEGIN_CXX
>+
>+#ifdef HAVE_GLIB
>+#if GLIB_CHECK_VERSION(2,32,0)
>+
>+GVariantCXX HashTable2Variant(const GHashTable *hashTable) throw ()
>+{
>+    GVariantBuilder builder;
>+    GHashTableIter iter;
>+    const gchar *key;
>+    GVariant *value;
>+
>+    if (!hashTable) {
>+        return GVariantCXX();
>+    }
>+
>+    g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
>+
>+    g_hash_table_iter_init(&iter, (GHashTable *)hashTable);
>+    while (g_hash_table_iter_next(&iter, (gpointer *)&key, (gpointer *)&value)) {
>+        g_variant_builder_add(&builder, "{sv}", key, g_variant_ref(value));
>+    }
>+    GVariantStealCXX variant(g_variant_builder_end(&builder));
>+    return variant;
>+}
>+
>+GHashTableCXX Variant2HashTable(GVariant *variant) throw ()
>+{
>+    if (!variant) {
>+        return GHashTableCXX();
>+    }
>+
>+    GVariantIter iter;
>+    GVariant *value;
>+    gchar *key;
>+
>+    GHashTableStealCXX hashTable(g_hash_table_new_full(g_str_hash,
>+                                                       g_str_equal,
>+                                                       g_free,
>+                                                       (GDestroyNotify)g_variant_unref));
>+    g_variant_iter_init(&iter, variant);
>+    while (g_variant_iter_next(&iter, "{sv}", &key, &value)) {
>+        g_hash_table_insert(hashTable, g_strdup(key), g_variant_ref(value));
>+    }
>+    return hashTable;
>+}
>+
>+#endif // GLIB_CHECK_VERSION
>+#endif // HAVE_GLIB
>+
>+SE_END_CXX
>diff --git a/src/syncevo/GVariantSupport.h b/src/syncevo/GVariantSupport.h
>new file mode 100644
>index 0000000..3890004
>--- /dev/null
>+++ b/src/syncevo/GVariantSupport.h
>@@ -0,0 +1,53 @@
>+/*
>+ * Copyright (C) 2014 Intel Corporation
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) version 3.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>+ * 02110-1301  USA
>+ */
>+
>+#ifndef INCL_GVARIANT_SUPPORT
>+#define INCL_GVARIANT_SUPPORT
>+
>+#include <syncevo/GLibSupport.h>
>+
>+#ifdef HAVE_GLIB
>+SE_GLIB_TYPE(GVariant, g_variant)
>+#endif
>+
>+#include <syncevo/declarations.h>
>+SE_BEGIN_CXX
>+
>+#ifdef HAVE_GLIB
>+
>+// G_VARIANT_TYPE_VARDICT was introduced in glib 2.32
>+#if GLIB_CHECK_VERSION(2,32,0)
>+
>+// Originally from google-oauth2-example.c, which is also under LGPL
>+// 2.1, or any later version.
>+GVariantCXX HashTable2Variant(const GHashTable *hashTable) throw ();
>+
>+/**
>+ * The created GHashTable maps strings to GVariants which are
>+ * reference counted, so when adding or setting values, use g_variant_ref_sink(g_variant_new_...()).
>+ */
>+GHashTableCXX Variant2HashTable(GVariant *variant) throw ();
>+
>+#endif // GLIB_CHECK_VERSION
>+
>+#endif // HAVE_GLIB
>+
>+SE_END_CXX
>+
>+#endif // INCL_GVARIANT_SUPPORT
>diff --git a/src/syncevo/syncevo.am b/src/syncevo/syncevo.am
>index 5ecb561..29eeddb 100644
>--- a/src/syncevo/syncevo.am
>+++ b/src/syncevo/syncevo.am
>@@ -39,6 +39,8 @@ src_syncevo_sources = \
>   src/syncevo/eds_abi_wrapper.cpp \
>   src/syncevo/GLibSupport.h \
>   src/syncevo/GLibSupport.cpp \
>+  src/syncevo/GVariantSupport.h \
>+  src/syncevo/GVariantSupport.cpp \
>   src/syncevo/ThreadSupport.h \
>   \
>   src/syncevo/SyncML.h \
>-- 
>2.1.0
>

Actions: View | Splinter Review

Attachments on bug 83396: 105605 | 105737 | 105906 | 105907 | 105908 | 105909 | 105910 | 105911 | 105912 | 105913 | 105914 | 105915 | 105955 | 105956 | 105981

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.