Merge into trunk : throw-for-unowned-types-lp1672657 : Code : Unity API

Status:

Merged

Approved by:

Pete Woods on 2017-03-17

Approved revision:

277

Merged at revision:

278

Proposed branch:

lp:~pete-woods/unity-api/throw-for-unowned-types-lp1672657

Merge into:

lp:unity-api

Diff against target:

79 lines (+26/-9)

2 files modified

include/unity/util/GObjectMemory.h (+9/-5)
test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp (+17/-4)

To merge this branch:

bzr merge lp:~pete-woods/unity-api/throw-for-unowned-types-lp1672657

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
James Henstridge		2017-03-16	Approve on 2017-03-17
Unity8 CI Bot	continuous-integration		Approve on 2017-03-17
Unity Team		2017-03-16	Pending
Review via email: mp+320059@code.launchpad.net

Commit message

unity::util - unique_gobject and share_gobject now throw for floating references

Description of the change

unity::util - unique_gobject and share_gobject now throw for floating references

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-16:

#

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

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2017-03-17:

#

Looks good. I think it'd be acceptable to have make_gobject() handle floating references though rather than throwing though: we know that we can't invalidate anyone else's pointers here.

lp:~pete-woods/unity-api/throw-for-unowned-types-lp1672657 updated on 2017-03-17

277. By Pete Woods on 2017-03-17: Allow make_gobject to construct initially unowned objects

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-17:

#

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

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2017-03-17:

#

Looks good.

review: Approve

 === modified file 'include/unity/util/GObjectMemory.h'
 --- include/unity/util/GObjectMemory.h	2017-02-23 11:18:54 +0000
 +++ include/unity/util/GObjectMemory.h	2017-03-17 10:20:36 +0000
@@ -33,11 +33,11 @@
  namespace
+ {
--inline static void sink_floating_gobject(gpointer t)
++inline static void check_floating_gobject(gpointer t)
+ {
--    if (t != nullptr && g_object_is_floating(G_OBJECT(t)))
++    if (G_IS_OBJECT(t) && g_object_is_floating(G_OBJECT(t)))
+     {
--        g_object_ref_sink(t);
++        throw std::invalid_argument("cannot manage floating GObject reference - call g_object_ref_sink(o) first");
+     }
+ }
@@ -81,7 +81,7 @@
  template<typename T>
  inline std::unique_ptr<T, GObjectDeleter> unique_gobject(T* ptr)
+ {
--    sink_floating_gobject(ptr);
++    check_floating_gobject(ptr);
      GObjectDeleter d;
      return std::unique_ptr<T, GObjectDeleter>(ptr, d);
+ }
@@ -101,7 +101,7 @@
  template<typename T>
  inline std::shared_ptr<T> share_gobject(T* ptr)
+ {
--    sink_floating_gobject(ptr);
++    check_floating_gobject(ptr);
      GObjectDeleter d;
      return std::shared_ptr<T>(ptr, d);
+ }
@@ -120,6 +120,10 @@
  inline std::unique_ptr<T, GObjectDeleter> make_gobject(GType object_type, const gchar *first_property_name, Args&&... args)
+ {
      gpointer ptr = g_object_new(object_type, first_property_name, std::forward<Args>(args)...);
++    if (G_IS_OBJECT(ptr) && g_object_is_floating(ptr))
++    {
++        g_object_ref_sink(ptr);
++    }
      return unique_gobject(G_TYPE_CHECK_INSTANCE_CAST(ptr, object_type, T));
+ }
 === modified file 'test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp'
 --- test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp	2017-02-23 11:18:54 +0000
 +++ test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp	2017-03-17 10:20:36 +0000
@@ -276,10 +276,23 @@
  TEST_F(GObjectMemoryTest, floating)
+ {
--    GObject* o = G_OBJECT(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
--    auto u = unique_gobject<GObject>(o);
--    // it should have sunk the reference
--    EXPECT_TRUE(g_object_is_floating(u.get()) == FALSE);
++    {
++        auto o = G_INITIALLY_UNOWNED(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
++        EXPECT_THROW(unique_gobject(o), invalid_argument);
++        g_object_ref_sink(G_OBJECT(o));
++        unique_gobject(o);
++    }
++    {
++        auto o = G_INITIALLY_UNOWNED(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
++        EXPECT_THROW(unique_gobject(o), invalid_argument);
++        g_object_ref_sink(G_OBJECT(o));
++        unique_gobject(o);
++    }
++
++    {
++        auto o = make_gobject<GInitiallyUnowned>(G_TYPE_INITIALLY_UNOWNED, nullptr);
++        EXPECT_FALSE(g_object_is_floating(o.get()));
++    }
+ }
  TEST_F(GObjectMemoryTest, move)

Unity API

Merge lp:~pete-woods/unity-api/throw-for-unowned-types-lp1672657 into lp:unity-api

Commit message

Description of the change

Preview Diff

Subscribers