Merge lp:~pete-woods/unity-api/handle-null-quietly into lp:unity-api

[Pete Woods]

unity::util - Make Glib and GObject memory management utilities handle NULL quietly.

unity::util - Make Glib and GObject memory management utilities handle NULL quietly.

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:274
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~pete-woods/unity-api/handle-null-quietly/+merge/317803/+edit-commit-message

https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/140/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4140/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4168/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/140/rebuild

review: Needs Fixing (continuous-integration)

[lp:~pete-woods/unity-api/handle-null-quietly]

275. By Pete Woods on 2017-02-21

Fix changelog

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:275
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~pete-woods/unity-api/handle-null-quietly/+merge/317803/+edit-commit-message

https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/141/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4142/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4170/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/141/rebuild

review: Needs Fixing (continuous-integration)

[lp:~pete-woods/unity-api/handle-null-quietly]

276. By Pete Woods on 2017-02-21

Really fix the changelog this time

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:276
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/143/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4147
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4175
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4012
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4012/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4012
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4012/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4012
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4012/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4012
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4012/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4012
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4012/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4012
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4012/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/143/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

The idea looks sound, code works as-is. Optional comments inline

review: Approve

[lp:~pete-woods/unity-api/handle-null-quietly]

277. By Pete Woods on 2017-02-23

Trying out the deleter methods as charles suggested, making CRITICAL be a fatal error so we know if the deleter logic worked (g_variant_unref logs CRITICAL on null)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:277
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/144/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4182
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4210
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4047
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4047
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4047
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4047
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4047
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4047
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4047/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/144/rebuild

review: Approve (continuous-integration)

[Pete Woods (pete-woods)]

Well clearly my paranoia with the == FALSE thing was based on madness, and we have a successful build on powerpc without it.

=== modified file 'debian/changelog'
--- debian/changelog	2017-02-17 14:12:04 +0000
+++ debian/changelog	2017-02-23 11:19:18 +0000
@@ -1,3 +1,11 @@
+unity-api (8.4-0ubuntu1) UNRELEASED; urgency=medium
+
+  [ Pete Woods ]
+  * unity::util - Make Glib and GObject memory management
+    utilities handle NULL quietly.
+
+ -- Pete <[e-mail]>  Tue, 21 Feb 2017 13:11:57 +0000
+
 unity-api (8.3+17.04.20170217-0ubuntu1) zesty; urgency=medium
 
   [ Pete ]

=== modified file 'include/unity/util/GObjectMemory.h'
--- include/unity/util/GObjectMemory.h	2017-01-19 13:49:43 +0000
+++ include/unity/util/GObjectMemory.h	2017-02-23 11:19:18 +0000
@@ -59,8 +59,10 @@
 {
     void operator()(gpointer ptr)
     {
-        g_return_if_fail(G_IS_OBJECT(ptr));
-        g_object_unref(ptr);
+        if (G_IS_OBJECT(ptr))
+        {
+            g_object_unref(ptr);
+        }
     }
 };
 

=== modified file 'include/unity/util/GlibMemory.h'
--- include/unity/util/GlibMemory.h	2017-02-17 13:40:58 +0000
+++ include/unity/util/GlibMemory.h	2017-02-23 11:19:18 +0000
@@ -28,18 +28,33 @@
 namespace util
 {
 
+template<typename T, typename D>
+struct GlibDeleter
+{
+    D _d;
+
+    void operator()(T* ptr)
+    {
+        if (ptr != nullptr)
+        {
+            _d(ptr);
+        }
+    }
+};
+
 #pragma push_macro("G_DEFINE_AUTOPTR_CLEANUP_FUNC")
 #undef G_DEFINE_AUTOPTR_CLEANUP_FUNC
 #define G_DEFINE_AUTOPTR_CLEANUP_FUNC(TypeName, func) \
+typedef GlibDeleter<TypeName, decltype(&func)> TypeName##Deleter; \
 typedef std::shared_ptr<TypeName> TypeName##SPtr; \
 inline TypeName##SPtr share_glib(TypeName* ptr) \
 { \
-    return TypeName##SPtr(ptr, &func); \
+    return TypeName##SPtr(ptr, TypeName##Deleter{&func}); \
 } \
-typedef std::unique_ptr<TypeName, decltype(&func)> TypeName##UPtr; \
+typedef std::unique_ptr<TypeName, TypeName##Deleter> TypeName##UPtr; \
 inline TypeName##UPtr unique_glib(TypeName* ptr) \
 { \
-    return TypeName##UPtr(ptr, &func); \
+    return TypeName##UPtr(ptr, TypeName##Deleter{&func}); \
 }
 
 #pragma push_macro("G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC")

=== modified file 'test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp'
--- test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp	2017-01-19 13:49:43 +0000
+++ test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp	2017-02-23 11:19:18 +0000
@@ -174,6 +174,11 @@
 class GObjectMemoryTest: public testing::Test
 {
 protected:
+    static void SetUpTestCase()
+    {
+        g_log_set_always_fatal((GLogLevelFlags) (G_LOG_LEVEL_CRITICAL | G_LOG_FLAG_FATAL));
+    }
+
     void SetUp() override
     {
         DELETED_OBJECTS.clear();
@@ -298,6 +303,7 @@
     auto u1 = unique_gobject(o1);
     auto u2 = unique_gobject<FooBar>(nullptr);
     auto u3 = unique_gobject(o3);
+    auto u4 = unique_gobject((FooBar *) NULL);
 
     EXPECT_TRUE(!u1);
     EXPECT_TRUE(!u2);

=== modified file 'test/gtest/unity/util/GlibMemory/GlibMemory_test.cpp'
--- test/gtest/unity/util/GlibMemory/GlibMemory_test.cpp	2017-02-17 13:40:58 +0000
+++ test/gtest/unity/util/GlibMemory/GlibMemory_test.cpp	2017-02-23 11:19:18 +0000
@@ -37,6 +37,11 @@
 class GlibMemoryTest: public testing::Test
 {
 protected:
+    static void SetUpTestCase()
+    {
+        g_log_set_always_fatal((GLogLevelFlags) (G_LOG_LEVEL_CRITICAL | G_LOG_FLAG_FATAL));
+    }
+
     void SetUp() override
     {
         keys.clear();
@@ -108,6 +113,10 @@
         EXPECT_EQ(set<string>({"hello", "hash"}), keys);
         EXPECT_EQ(set<string>({"there", "world"}), values);
     }
+
+    {
+        auto emptyVariant = unique_glib((GVariant*) NULL);
+    }
 }
 
 TEST_F(GlibMemoryTest, Share)
@@ -132,6 +141,10 @@
         EXPECT_EQ(set<string>({"hello", "hash"}), keys);
         EXPECT_EQ(set<string>({"there", "world"}), values);
     }
+
+    {
+        auto emptyVariant = share_glib((GVariant*) NULL);
+    }
 }
 
 }