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

Proposed by Pete Woods on 2017-03-16
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
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

To post a comment you must log in.
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)
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 on 2017-03-17

Allow make_gobject to construct initially unowned objects

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)
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
1=== modified file 'include/unity/util/GObjectMemory.h'
2--- include/unity/util/GObjectMemory.h 2017-02-23 11:18:54 +0000
3+++ include/unity/util/GObjectMemory.h 2017-03-17 10:20:36 +0000
4@@ -33,11 +33,11 @@
5 namespace
6 {
7
8-inline static void sink_floating_gobject(gpointer t)
9+inline static void check_floating_gobject(gpointer t)
10 {
11- if (t != nullptr && g_object_is_floating(G_OBJECT(t)))
12+ if (G_IS_OBJECT(t) && g_object_is_floating(G_OBJECT(t)))
13 {
14- g_object_ref_sink(t);
15+ throw std::invalid_argument("cannot manage floating GObject reference - call g_object_ref_sink(o) first");
16 }
17 }
18
19@@ -81,7 +81,7 @@
20 template<typename T>
21 inline std::unique_ptr<T, GObjectDeleter> unique_gobject(T* ptr)
22 {
23- sink_floating_gobject(ptr);
24+ check_floating_gobject(ptr);
25 GObjectDeleter d;
26 return std::unique_ptr<T, GObjectDeleter>(ptr, d);
27 }
28@@ -101,7 +101,7 @@
29 template<typename T>
30 inline std::shared_ptr<T> share_gobject(T* ptr)
31 {
32- sink_floating_gobject(ptr);
33+ check_floating_gobject(ptr);
34 GObjectDeleter d;
35 return std::shared_ptr<T>(ptr, d);
36 }
37@@ -120,6 +120,10 @@
38 inline std::unique_ptr<T, GObjectDeleter> make_gobject(GType object_type, const gchar *first_property_name, Args&&... args)
39 {
40 gpointer ptr = g_object_new(object_type, first_property_name, std::forward<Args>(args)...);
41+ if (G_IS_OBJECT(ptr) && g_object_is_floating(ptr))
42+ {
43+ g_object_ref_sink(ptr);
44+ }
45 return unique_gobject(G_TYPE_CHECK_INSTANCE_CAST(ptr, object_type, T));
46 }
47
48
49=== modified file 'test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp'
50--- test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp 2017-02-23 11:18:54 +0000
51+++ test/gtest/unity/util/GObjectMemory/GObjectMemory_test.cpp 2017-03-17 10:20:36 +0000
52@@ -276,10 +276,23 @@
53
54 TEST_F(GObjectMemoryTest, floating)
55 {
56- GObject* o = G_OBJECT(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
57- auto u = unique_gobject<GObject>(o);
58- // it should have sunk the reference
59- EXPECT_TRUE(g_object_is_floating(u.get()) == FALSE);
60+ {
61+ auto o = G_INITIALLY_UNOWNED(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
62+ EXPECT_THROW(unique_gobject(o), invalid_argument);
63+ g_object_ref_sink(G_OBJECT(o));
64+ unique_gobject(o);
65+ }
66+ {
67+ auto o = G_INITIALLY_UNOWNED(g_object_new(G_TYPE_INITIALLY_UNOWNED, nullptr));
68+ EXPECT_THROW(unique_gobject(o), invalid_argument);
69+ g_object_ref_sink(G_OBJECT(o));
70+ unique_gobject(o);
71+ }
72+
73+ {
74+ auto o = make_gobject<GInitiallyUnowned>(G_TYPE_INITIALLY_UNOWNED, nullptr);
75+ EXPECT_FALSE(g_object_is_floating(o.get()));
76+ }
77 }
78
79 TEST_F(GObjectMemoryTest, move)

Subscribers

People subscribed via source and target branches

to all changes: