Message ID | 20250417113539.1137936-1-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Quoting Barnabás Pőcze (2025-04-17 12:35:37) > In order to be able to copy the storage as one unit, regardless of > which member is active give a name to the union member. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/controls.h | 6 +++--- > src/libcamera/controls.cpp | 17 ++++++++--------- > 2 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 4bfe9615c..1dc6ccffa 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -238,9 +238,9 @@ private: > bool isArray_; > std::size_t numElements_ : 32; > union { > - uint64_t value_; > - void *storage_; > - }; > + uint64_t internal; > + void *external; > + } storage_; > > void release(); > void set(ControlType type, bool isArray, const void *data, > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 70f6f6092..d384e1ef7 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -10,6 +10,7 @@ > #include <sstream> > #include <string.h> > #include <string> > +#include <utility> > > #include <libcamera/base/log.h> > #include <libcamera/base/utils.h> > @@ -122,10 +123,8 @@ void ControlValue::release() > { > std::size_t size = numElements_ * ControlValueSize[type_]; > > - if (size > sizeof(value_)) { > - delete[] reinterpret_cast<uint8_t *>(storage_); > - storage_ = nullptr; > - } > + if (size > sizeof(storage_.internal)) > + delete[] reinterpret_cast<uint8_t *>(std::exchange(storage_.external, nullptr)); > } > > ControlValue::~ControlValue() > @@ -192,9 +191,9 @@ ControlValue &ControlValue::operator=(const ControlValue &other) > Span<const uint8_t> ControlValue::data() const > { > std::size_t size = numElements_ * ControlValueSize[type_]; > - const uint8_t *data = size > sizeof(value_) > - ? reinterpret_cast<const uint8_t *>(storage_) > - : reinterpret_cast<const uint8_t *>(&value_); > + const uint8_t *data = size > sizeof(storage_.internal) > + ? reinterpret_cast<const uint8_t *>(storage_.external) > + : reinterpret_cast<const uint8_t *>(&storage_.internal); > return { data, size }; > } > > @@ -391,8 +390,8 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen > if (oldSize == newSize) > return; > > - if (newSize > sizeof(value_)) > - storage_ = reinterpret_cast<void *>(new uint8_t[newSize]); > + if (newSize > sizeof(storage_.internal)) > + storage_.external = new uint8_t[newSize]; > } > > /** > -- > 2.49.0 >
Hi Barnabás On Thu, Apr 17, 2025 at 01:35:37PM +0200, Barnabás Pőcze wrote: > In order to be able to copy the storage as one unit, regardless of > which member is active give a name to the union member. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > include/libcamera/controls.h | 6 +++--- > src/libcamera/controls.cpp | 17 ++++++++--------- > 2 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 4bfe9615c..1dc6ccffa 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -238,9 +238,9 @@ private: > bool isArray_; > std::size_t numElements_ : 32; > union { > - uint64_t value_; > - void *storage_; > - }; > + uint64_t internal; > + void *external; We could nitpick on names here, but I won't :) Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > + } storage_; > > void release(); > void set(ControlType type, bool isArray, const void *data, > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 70f6f6092..d384e1ef7 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -10,6 +10,7 @@ > #include <sstream> > #include <string.h> > #include <string> > +#include <utility> > > #include <libcamera/base/log.h> > #include <libcamera/base/utils.h> > @@ -122,10 +123,8 @@ void ControlValue::release() > { > std::size_t size = numElements_ * ControlValueSize[type_]; > > - if (size > sizeof(value_)) { > - delete[] reinterpret_cast<uint8_t *>(storage_); > - storage_ = nullptr; > - } > + if (size > sizeof(storage_.internal)) > + delete[] reinterpret_cast<uint8_t *>(std::exchange(storage_.external, nullptr)); > } > > ControlValue::~ControlValue() > @@ -192,9 +191,9 @@ ControlValue &ControlValue::operator=(const ControlValue &other) > Span<const uint8_t> ControlValue::data() const > { > std::size_t size = numElements_ * ControlValueSize[type_]; > - const uint8_t *data = size > sizeof(value_) > - ? reinterpret_cast<const uint8_t *>(storage_) > - : reinterpret_cast<const uint8_t *>(&value_); > + const uint8_t *data = size > sizeof(storage_.internal) > + ? reinterpret_cast<const uint8_t *>(storage_.external) > + : reinterpret_cast<const uint8_t *>(&storage_.internal); > return { data, size }; > } > > @@ -391,8 +390,8 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen > if (oldSize == newSize) > return; > > - if (newSize > sizeof(value_)) > - storage_ = reinterpret_cast<void *>(new uint8_t[newSize]); > + if (newSize > sizeof(storage_.internal)) > + storage_.external = new uint8_t[newSize]; > } > > /** > -- > 2.49.0 >
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 4bfe9615c..1dc6ccffa 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -238,9 +238,9 @@ private: bool isArray_; std::size_t numElements_ : 32; union { - uint64_t value_; - void *storage_; - }; + uint64_t internal; + void *external; + } storage_; void release(); void set(ControlType type, bool isArray, const void *data, diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 70f6f6092..d384e1ef7 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -10,6 +10,7 @@ #include <sstream> #include <string.h> #include <string> +#include <utility> #include <libcamera/base/log.h> #include <libcamera/base/utils.h> @@ -122,10 +123,8 @@ void ControlValue::release() { std::size_t size = numElements_ * ControlValueSize[type_]; - if (size > sizeof(value_)) { - delete[] reinterpret_cast<uint8_t *>(storage_); - storage_ = nullptr; - } + if (size > sizeof(storage_.internal)) + delete[] reinterpret_cast<uint8_t *>(std::exchange(storage_.external, nullptr)); } ControlValue::~ControlValue() @@ -192,9 +191,9 @@ ControlValue &ControlValue::operator=(const ControlValue &other) Span<const uint8_t> ControlValue::data() const { std::size_t size = numElements_ * ControlValueSize[type_]; - const uint8_t *data = size > sizeof(value_) - ? reinterpret_cast<const uint8_t *>(storage_) - : reinterpret_cast<const uint8_t *>(&value_); + const uint8_t *data = size > sizeof(storage_.internal) + ? reinterpret_cast<const uint8_t *>(storage_.external) + : reinterpret_cast<const uint8_t *>(&storage_.internal); return { data, size }; } @@ -391,8 +390,8 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen if (oldSize == newSize) return; - if (newSize > sizeof(value_)) - storage_ = reinterpret_cast<void *>(new uint8_t[newSize]); + if (newSize > sizeof(storage_.internal)) + storage_.external = new uint8_t[newSize]; } /**
In order to be able to copy the storage as one unit, regardless of which member is active give a name to the union member. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/controls.h | 6 +++--- src/libcamera/controls.cpp | 17 ++++++++--------- 2 files changed, 11 insertions(+), 12 deletions(-)