[libcamera-devel] [PATCH v1.1 17/23] libcamera: control: Deep-copy control values

[Jacopo Mondi]

Implement operator=() and copy constructor for the ControlValue class,
deep-copying the content of the 'other' ControlValue by using
ControlValue::set() operation which, in case of compound ControlValue,
guarantees proper memory allocation and data copy.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
Valgrind reports the usage of unitialized value in ControlValue::relase()

This was caused by the copy constructor calling set<>() which calls release()
before the type of the control values was initialized (we're at construction
time)

Fix this by initializing type before calling operator=()

--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -201,6 +201,11 @@ void ControlValue::copyValue(const ControlValue &other)
  */
 ControlValue::ControlValue(const ControlValue &other)
 {
+       /*
+        * Initialze the type: operator=() calls set() which calls release()
+        * which inspects the control value type. Be safe and set it to none.
+        */
+       type_ = ControlTypeNone;
        *this = other;
 }

---
 include/libcamera/controls.h |  5 +++
 src/libcamera/controls.cpp   | 67 ++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index bdbdb213528d..d13144b69198 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -43,6 +43,9 @@ public:
 	ControlValue(Span<float> &values);
 	~ControlValue();

+	ControlValue(const ControlValue &other);
+	ControlValue &operator=(const ControlValue &other);
+
 	ControlType type() const { return type_; }
 	bool isNone() const { return type_ == ControlTypeNone; }
 	std::size_t numElements() const { return numElements_; }
@@ -81,6 +84,8 @@ private:
 	std::size_t numElements_;

 	void release();
+	template<typename T>
+	void copyValue(const ControlValue &source);
 	bool compareElement(const ControlValue &other) const;
 	bool compareElement(const ControlValue &other, unsigned int i) const;
 	std::string elemToString(unsigned int i) const;
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index c311ab1d6af1..ddf666040ae7 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -189,6 +189,73 @@ ControlValue::~ControlValue()
 	release();
 }

+template<typename T>
+void ControlValue::copyValue(const ControlValue &other)
+{
+	set<T>(other.get<T>());
+}
+
+/**
+ * \brief Contruct a ControlValue with the content of \a other
+ * \param[in] other The ControlValue to copy content from
+ */
+ControlValue::ControlValue(const ControlValue &other)
+{
+	/*
+	 * Initialze the type: operator=() calls set() which calls release()
+	 * which inspects the control value type. Be safe and set it to none.
+	 */
+	type_ = ControlTypeNone;
+	*this = other;
+}
+
+/**
+ * \brief Replace the content of the ControlValue with the one of \a other
+ * \param[in] other The ControlValue to copy content from
+ *
+ * Deep-copy the content of \a other into the ControlValue by reserving memory
+ * and copy data there in case \a other transports arrays of values in one of
+ * its pointer data members.
+ *
+ * \return The ControlValue with its content replaced with the one of \a other
+ */
+ControlValue &ControlValue::operator=(const ControlValue &other)
+{
+	switch (other.type_) {
+	case ControlTypeBool:
+		copyValue<bool>(other);
+		break;
+	case ControlTypeInteger32:
+		copyValue<int32_t>(other);
+		break;
+	case ControlTypeInteger64:
+		copyValue<int64_t>(other);
+		break;
+	case ControlTypeFloat:
+		copyValue<float>(other);
+		break;
+	case ControlTypeCompoundBool:
+		copyValue<Span<bool>>(other);
+		break;
+	case ControlTypeCompoundInt32:
+		copyValue<Span<int32_t>>(other);
+		break;
+	case ControlTypeCompoundInt64:
+		copyValue<Span<int64_t>>(other);
+		break;
+	case ControlTypeCompoundFloat:
+		copyValue<Span<float>>(other);
+		break;
+	default:
+		break;
+	}
+
+	type_ = other.type_;
+	numElements_ = other.numElements_;
+
+	return *this;
+}
+
 /**
  * \fn ControlValue::type()
  * \brief Retrieve the data type of the value
--
2.24.1