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

Proposed by Pete Woods
Status: Merged
Approved by: Pete Woods
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
Reviewer Review Type Date Requested Status
James Henstridge Approve
Unity8 CI Bot continuous-integration Approve
Unity Team 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

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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

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 :

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.

277. By Pete Woods

Allow make_gobject to construct initially unowned objects

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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

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 :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
33namespace33namespace
34{34{
3535
36inline static void sink_floating_gobject(gpointer t)36inline static void check_floating_gobject(gpointer t)
37{37{
38 if (t != nullptr && g_object_is_floating(G_OBJECT(t)))38 if (G_IS_OBJECT(t) && g_object_is_floating(G_OBJECT(t)))
39 {39 {
40 g_object_ref_sink(t);40 throw std::invalid_argument("cannot manage floating GObject reference - call g_object_ref_sink(o) first");
41 }41 }
42}42}
4343
@@ -81,7 +81,7 @@
81template<typename T>81template<typename T>
82inline std::unique_ptr<T, GObjectDeleter> unique_gobject(T* ptr)82inline std::unique_ptr<T, GObjectDeleter> unique_gobject(T* ptr)
83{83{
84 sink_floating_gobject(ptr);84 check_floating_gobject(ptr);
85 GObjectDeleter d;85 GObjectDeleter d;
86 return std::unique_ptr<T, GObjectDeleter>(ptr, d);86 return std::unique_ptr<T, GObjectDeleter>(ptr, d);
87}87}
@@ -101,7 +101,7 @@
101template<typename T>101template<typename T>
102inline std::shared_ptr<T> share_gobject(T* ptr)102inline std::shared_ptr<T> share_gobject(T* ptr)
103{103{
104 sink_floating_gobject(ptr);104 check_floating_gobject(ptr);
105 GObjectDeleter d;105 GObjectDeleter d;
106 return std::shared_ptr<T>(ptr, d);106 return std::shared_ptr<T>(ptr, d);
107}107}
@@ -120,6 +120,10 @@
120inline std::unique_ptr<T, GObjectDeleter> make_gobject(GType object_type, const gchar *first_property_name, Args&&... args)120inline std::unique_ptr<T, GObjectDeleter> make_gobject(GType object_type, const gchar *first_property_name, Args&&... args)
121{121{
122 gpointer ptr = g_object_new(object_type, first_property_name, std::forward<Args>(args)...);122 gpointer ptr = g_object_new(object_type, first_property_name, std::forward<Args>(args)...);
123 if (G_IS_OBJECT(ptr) && g_object_is_floating(ptr))
124 {
125 g_object_ref_sink(ptr);
126 }
123 return unique_gobject(G_TYPE_CHECK_INSTANCE_CAST(ptr, object_type, T));127 return unique_gobject(G_TYPE_CHECK_INSTANCE_CAST(ptr, object_type, T));
124}128}
125129
126130
=== 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 @@
276276
277TEST_F(GObjectMemoryTest, floating)277TEST_F(GObjectMemoryTest, floating)
278{278{
279 GObject* o = G_OBJECT(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));279 {
280 auto u = unique_gobject<GObject>(o);280 auto o = G_INITIALLY_UNOWNED(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
281 // it should have sunk the reference281 EXPECT_THROW(unique_gobject(o), invalid_argument);
282 EXPECT_TRUE(g_object_is_floating(u.get()) == FALSE);282 g_object_ref_sink(G_OBJECT(o));
283 unique_gobject(o);
284 }
285 {
286 auto o = G_INITIALLY_UNOWNED(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
287 EXPECT_THROW(unique_gobject(o), invalid_argument);
288 g_object_ref_sink(G_OBJECT(o));
289 unique_gobject(o);
290 }
291
292 {
293 auto o = make_gobject<GInitiallyUnowned>(G_TYPE_INITIALLY_UNOWNED, nullptr);
294 EXPECT_FALSE(g_object_is_floating(o.get()));
295 }
283}296}
284297
285TEST_F(GObjectMemoryTest, move)298TEST_F(GObjectMemoryTest, move)

Subscribers

People subscribed via source and target branches

to all changes: