[vlc-devel] [PATCH] qt: casting to children type in parent constructor is UB

Pierre Lamot pierre at videolabs.io
Mon Apr 29 14:57:58 CEST 2019


  children object isn't built yet.

  Fixes #22187
---
 modules/gui/qt/adapters/var_common_p.hpp | 18 +++++++
 modules/gui/qt/adapters/variables.hpp    | 60 ++++++++++++++++--------
 2 files changed, 59 insertions(+), 19 deletions(-)

diff --git a/modules/gui/qt/adapters/var_common_p.hpp b/modules/gui/qt/adapters/var_common_p.hpp
index 47f8e89ca1..7ac324739b 100644
--- a/modules/gui/qt/adapters/var_common_p.hpp
+++ b/modules/gui/qt/adapters/var_common_p.hpp
@@ -51,6 +51,8 @@ public:
         assert(false);
     }
 
+    virtual void clear() {}
+
     operator bool() const {
         return get() != nullptr;
     }
@@ -81,6 +83,10 @@ public:
         m_object.reset(vout, hold);
     }
 
+    void clear() override {
+        m_object.reset(NULL, false);
+    }
+
 private:
     static void obj_hold( vout_thread_t* obj ) { vout_Hold(obj); }
     static void obj_release( vout_thread_t* obj ) { vout_Release(obj); }
@@ -108,6 +114,10 @@ public:
         m_object = p_intf;
     }
 
+    void clear() override {
+        m_object = NULL;
+    }
+
 private:
     intf_thread_t* m_object;
 };
@@ -133,6 +143,10 @@ public:
         m_object.reset(aout, hold);
     }
 
+    void clear() override {
+        m_object.reset(NULL, false);
+    }
+
 private:
     static void obj_hold( audio_output_t* obj ) { aout_Hold(obj); }
     static void obj_release( audio_output_t* obj ) { aout_Release(obj); }
@@ -157,6 +171,10 @@ public:
         m_object = obj;
     }
 
+    void clear() override {
+        m_object = NULL;
+    }
+
 private:
     vlc_object_t *m_object;
 };
diff --git a/modules/gui/qt/adapters/variables.hpp b/modules/gui/qt/adapters/variables.hpp
index 710880cf23..c500187c50 100644
--- a/modules/gui/qt/adapters/variables.hpp
+++ b/modules/gui/qt/adapters/variables.hpp
@@ -112,39 +112,26 @@ public:
         , m_object(new VLCObjectHolderImpl<T>(nullptr))
         , m_property(property)
     {
-        resetObject<T>(object);
-        Derived* derived = static_cast<Derived*>(this);
-        connect(derived, &Derived::valueChangedInternal, this, &QVLCVariable<Derived, BaseType>::onValueChangedInternal, Qt::QueuedConnection);
     }
 
     virtual ~QVLCVariable()
     {
-        if (m_object->get())
-        {
-            Derived* derived = static_cast<Derived*>(this);
-            var_DelCallback(m_object->get(), qtu(m_property), value_modified, derived);
-            var_Destroy(m_object->get(), qtu(m_property));
-        }
+        assert(m_object->get() == nullptr);
     }
 
     ///change the object beeing observed
     template<typename T>
     void resetObject( T* object )
     {
-        Derived* derived = static_cast<Derived*>(this);
-        if (m_object->get()) {
-            var_DelCallback( m_object->get(), qtu(m_property), value_modified, derived );
-            var_Destroy(m_object->get(), qtu(m_property));
-        }
-
-        m_object->reset( object, true );
-        if (m_object->get())
+        clearObject();
+        if (object)
         {
+            m_object->reset( object, true );
             int type = var_Type(object, qtu(m_property));
             if (type == 0) //variable not found
             {
                 msg_Warn(m_object->get(), "variable %s not found in object", qtu(m_property));
-                m_object->reset((T*)nullptr);
+                m_object->clear();
                 return;
             }
             assert((type & VLC_VAR_CLASS) == VLCVarTypeTraits<BaseType>::var_type);
@@ -154,13 +141,24 @@ public:
                 BaseType value = VLCVarTypeTraits<BaseType>::fromValue(currentvalue);
                 if (m_value != value)
                 {
+                    Derived* derived = static_cast<Derived*>(this);
                     m_value = value;
                     emit derived->valueChanged( m_value );
                 }
             }
 
             var_Create(m_object->get(), qtu(m_property), VLCVarTypeTraits<BaseType>::var_type);
-            var_AddCallback(m_object->get(), qtu(m_property), value_modified, derived);
+            var_AddCallback(m_object->get(), qtu(m_property), value_modified, this);
+        }
+    }
+
+    void clearObject()
+    {
+        if (m_object->get())
+        {
+            var_DelCallback( m_object->get(), qtu(m_property), value_modified, this );
+            var_Destroy(m_object->get(), qtu(m_property));
+            m_object->clear();
         }
     }
 
@@ -221,6 +219,12 @@ public:
     QVLCBool(T* object, QString property, QObject* parent = nullptr)
         : QVLCVariable<QVLCBool, bool>(object, property, parent)
     {
+        resetObject<T>(object);
+        connect(this, &QVLCBool::valueChangedInternal, this, &QVLCBool::onValueChangedInternal, Qt::QueuedConnection);
+    }
+
+    ~QVLCBool() {
+        clearObject();
     }
 
 public slots:
@@ -241,6 +245,12 @@ public:
     QVLCString(T* object, QString property, QObject* parent = nullptr)
         : QVLCVariable<QVLCString, QString>(object, property, parent)
     {
+        resetObject<T>(object);
+        connect(this, &QVLCString::valueChangedInternal, this, &QVLCString::onValueChangedInternal, Qt::QueuedConnection);
+    }
+
+    ~QVLCString() {
+        clearObject();
     }
 
 public slots:
@@ -261,6 +271,12 @@ public:
     QVLCFloat(T* object, QString property, QObject* parent = nullptr)
         : QVLCVariable<QVLCFloat, float>(object, property, parent)
     {
+        resetObject<T>(object);
+        connect(this, &QVLCFloat::valueChangedInternal, this, &QVLCFloat::onValueChangedInternal, Qt::QueuedConnection);
+    }
+
+    ~QVLCFloat() {
+        clearObject();
     }
 
 public slots:
@@ -282,6 +298,12 @@ public:
     QVLCInteger(T* object, QString property, QObject* parent = nullptr)
         : QVLCVariable<QVLCInteger, int64_t>(object, property, parent)
     {
+        resetObject<T>(object);
+        connect(this, &QVLCInteger::valueChangedInternal, this, &QVLCInteger::onValueChangedInternal, Qt::QueuedConnection);
+    }
+
+    ~QVLCInteger() {
+        clearObject();
     }
 
 public slots:
-- 
2.17.1



More information about the vlc-devel mailing list