[vlc-devel] [PATCH v5] include: add C++ wrapper for C shared resources

Romain Vimont rom1v at videolabs.io
Thu Oct 11 16:26:18 CEST 2018


Add a helper to create RAII wrappers for C shared resources, which
automatically call the Hold() and Release() functions associated to the
raw pointer.

Just declare a new shared resource wrapper type:

    using InputItemPtr = vlc_shared_data_ptr_type(input_item_t,
                                                  input_item_Hold,
                                                  input_item_Release);

Then use this new type directly:

    input_item_t *ptr = /* ... */;
    InputItemPtr item(ptr);
    QString name = item->psz_name;
    InputItemPtr other = item; /* hold automatically */
---
Changes from v4:
 - expose flag to make holding optional in constructor and reset() (the
   behavior was inconsitent)
 - add ::vlc:: namespace prefix in the macro, so that it can be called
   from outside the vlc namespace
 - add tests

 include/vlc_cxx_helpers.hpp  | 137 +++++++++++++++++++++++++++++++++
 src/Makefile.am              |   4 +-
 src/test/shared_data_ptr.cpp | 144 +++++++++++++++++++++++++++++++++++
 3 files changed, 284 insertions(+), 1 deletion(-)
 create mode 100644 src/test/shared_data_ptr.cpp

diff --git a/include/vlc_cxx_helpers.hpp b/include/vlc_cxx_helpers.hpp
index 832a964e53..ed807977b1 100644
--- a/include/vlc_cxx_helpers.hpp
+++ b/include/vlc_cxx_helpers.hpp
@@ -122,6 +122,143 @@ inline std::unique_ptr<T[], void (*)(void*)> wrap_carray( T* ptr ) noexcept
 
 } // anonymous namespace
 
+///
+/// Wraps a C shared resource having associated Hold() and Release() functions
+//
+/// This is a RAII wrapper for C shared resources (which are manually managed by
+/// calling explicitly their Hold() and Release() functions).
+///
+/// The Hold() and Release() functions must accept exactly one parameter having
+/// type T* (the raw pointer type). Their return type is irrelevant.
+///
+/// To create a new shared resource wrapper type for my_type_t, simply declare:
+///
+///     using MyTypePtr =
+///         vlc_shared_data_ptr_type(my_type_t, my_type_Hold, my_type_Release);
+///
+/// Then use it to wrap a raw C pointer:
+///
+///     my_type_t *raw_ptr = /* ... */;
+///     MyTypePtr ptr(raw_ptr);
+
+// In C++17, the template declaration could be replaced by:
+//     template<typename T, auto HOLD, auto RELEASE>
+template <typename T, typename H, typename R, H HOLD, R RELEASE>
+class vlc_shared_data_ptr {
+    T *ptr = nullptr;
+
+public:
+    /* default implicit constructor */
+    vlc_shared_data_ptr() = default;
+
+    /**
+     * Wrap a shared resource.
+     *
+     * If the pointer is not nullptr, and hold is true, then the resource is
+     * hold (the caller shared ownership is preserved).
+     * If hold is false, then the caller transfers the ownership to this
+     * wrapper.
+     *
+     * \param ptr  the raw pointer (can be nullptr)
+     * \param hold whether the resource must be hold
+     */
+    explicit vlc_shared_data_ptr(T *ptr, bool hold = true)
+        : ptr(ptr)
+    {
+        if (ptr && hold)
+            HOLD(ptr);
+    }
+
+    vlc_shared_data_ptr(const vlc_shared_data_ptr &other)
+        : vlc_shared_data_ptr(other.ptr) {}
+
+    vlc_shared_data_ptr(vlc_shared_data_ptr &&other) noexcept
+        : ptr(other.ptr)
+    {
+        other.ptr = nullptr;
+    }
+
+    ~vlc_shared_data_ptr()
+    {
+        if (ptr)
+            RELEASE(ptr);
+    }
+
+    vlc_shared_data_ptr &operator=(const vlc_shared_data_ptr &other)
+    {
+        reset(other.ptr, true);
+        return *this;
+    }
+
+    vlc_shared_data_ptr &operator=(vlc_shared_data_ptr &&other) noexcept
+    {
+        reset(other.ptr, false);
+        other.ptr = nullptr;
+        return *this;
+    }
+
+    bool operator==(const vlc_shared_data_ptr &other) const
+    {
+        return ptr == other.ptr;
+    }
+
+    bool operator!=(const vlc_shared_data_ptr &other) const
+    {
+        return !(*this == other);
+    }
+
+    operator bool() const
+    {
+        return ptr;
+    }
+
+    T &operator*() const
+    {
+        return *ptr;
+    }
+
+    T *operator->() const
+    {
+        return ptr;
+    }
+
+    T *get() const
+    {
+        return ptr;
+    }
+
+    /**
+     * Reset the shared resource.
+     *
+     *     ptr.reset(rawptr, hold);
+     *
+     * is semantically equivalent to:
+     *
+     *     ptr = vlc_shared_data_ptr<...>(rawptr, hold);
+     *
+     * If the pointer is not nullptr, and hold is true, then the resource is
+     * hold (the caller shared ownership is preserved).
+     * If hold is false, then the caller transfers the ownership to this
+     * wrapper.
+     *
+     * \param ptr  the raw pointer (can be nullptr)
+     * \param hold whether the resource must be hold
+     */
+    void reset(T *newptr = nullptr, bool hold = true)
+    {
+        if (newptr && hold)
+            HOLD(newptr);
+        if (ptr)
+            RELEASE(ptr);
+        ptr = newptr;
+    }
+};
+
+// useful due to the unnecessarily complex template declaration before C++17
+#define vlc_shared_data_ptr_type(type, hold, release) \
+    ::vlc::vlc_shared_data_ptr<type, decltype(&hold), decltype(&release), \
+                               &hold, &release>
+
 #ifdef VLC_THREADS_H_
 
 namespace threads
diff --git a/src/Makefile.am b/src/Makefile.am
index c81339f961..5e28c49c2f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -530,7 +530,8 @@ check_PROGRAMS = \
 	test_xmlent \
 	test_headers \
 	test_mrl_helpers \
-	test_arrays
+	test_arrays \
+	test_shared_data_ptr
 
 TESTS = $(check_PROGRAMS) check_symbols
 
@@ -553,6 +554,7 @@ test_xmlent_SOURCES = test/xmlent.c
 test_headers_SOURCES = test/headers.c
 test_mrl_helpers_SOURCES = test/mrl_helpers.c
 test_arrays_SOURCES = test/arrays.c
+test_shared_data_ptr_SOURCES = test/shared_data_ptr.cpp
 
 AM_LDFLAGS = -no-install
 LDADD = libvlccore.la \
diff --git a/src/test/shared_data_ptr.cpp b/src/test/shared_data_ptr.cpp
new file mode 100644
index 0000000000..7c06720629
--- /dev/null
+++ b/src/test/shared_data_ptr.cpp
@@ -0,0 +1,144 @@
+#include <assert.h>
+#include <algorithm>
+#include <vector>
+#include <vlc_cxx_helpers.hpp>
+
+struct mockrc
+{
+    int count = 1;
+};
+
+static void mockrc_Hold(mockrc *p)
+{
+    assert(p->count > 0);
+    p->count++;
+}
+
+static void mockrc_Release(mockrc *p)
+{
+    assert(p->count > 0);
+    p->count--;
+}
+
+using MockRcPtr = vlc_shared_data_ptr_type(mockrc, mockrc_Hold, mockrc_Release);
+
+static void test_raii()
+{
+    mockrc mock;
+
+    {
+        MockRcPtr ptr(&mock, true);
+        assert(ptr);
+        assert(mock.count == 2);
+        assert(ptr->count == 2);
+
+        {
+            mockrc_Hold(&mock);
+            /* transfer the ownership */
+            MockRcPtr ptr2(&mock, false);
+            assert(ptr->count == 3);
+        }
+
+        assert(ptr->count == 2);
+    }
+
+    assert(mock.count == 1);
+}
+
+static void test_assignment()
+{
+    mockrc mock;
+
+    MockRcPtr ptr(&mock, false);
+    assert(mock.count == 1);
+
+    {
+        MockRcPtr ptr2 = ptr;
+        assert(mock.count == 2);
+
+        {
+            MockRcPtr ptr3 = std::move(ptr2);
+            assert(mock.count == 2);
+
+            /* assign a nullptr, this releases the previous one */
+            ptr3 = {};
+            assert(mock.count == 1);
+
+            ptr3 = MockRcPtr(&mock);
+            assert(mock.count == 2);
+
+            MockRcPtr ptr4(std::move(ptr3));
+            assert(mock.count == 2);
+        }
+        /* ptr4 is the only wrapper alive (ptr3 had been moved), count is
+         * decremented by 1 */
+        assert(mock.count == 1);
+    }
+
+    /* ptr2 had been moved, no decrement */
+    assert(mock.count == 1);
+
+    ptr = ptr; /* self-assignement should have no effect */
+    assert(mock.count == 1);
+}
+
+static void test_reset()
+{
+    mockrc mock;
+
+    {
+        MockRcPtr ptr(&mock, false);
+        assert(mock.count == 1);
+
+        /* hold once, release once, count is not changed */
+        ptr.reset(&mock, true);
+        assert(mock.count == 1);
+
+        mockrc_Hold(&mock);
+        ptr.reset(&mock, false);
+        assert(mock.count == 1);
+    }
+
+    /* ownership had transferred to ptr, which has been destroyed */
+    assert(mock.count == 0);
+}
+
+static void test_vector()
+{
+    mockrc mock;
+
+    {
+        std::vector<MockRcPtr> vec(10, MockRcPtr(&mock, false));
+        assert(mock.count == 10);
+
+        std::random_shuffle(vec.begin(), vec.end());
+        assert(mock.count == 10);
+
+        {
+            auto vec2 = vec;
+            assert(mock.count == 20);
+
+            vec2.push_back(MockRcPtr(&mock, true));
+            assert(mock.count == 21);
+        }
+
+        assert(mock.count == 10);
+
+        vec.erase(vec.begin() + 2, vec.begin() + 6);
+        assert(mock.count == 6);
+
+        vec.emplace_back(&mock, true);
+        assert(mock.count == 7);
+    }
+
+    /* ownership was transferred to vec constructor */
+    assert(mock.count == 0);
+}
+
+int main() {
+    test_raii();
+    test_assignment();
+    test_reset();
+    test_vector();
+    return 0;
+}
-- 
2.19.1



More information about the vlc-devel mailing list