| Message ID | 20210211133444.764808-7-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Feb 11, 2021 at 01:34:44PM +0000, Kieran Bingham wrote: > Prevent copying and assignment of MappedBuffer types, which could invoke > undesired unmapping of mapped memory. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> I think the commit message isn't 100% right. As explained in https://en.cppreference.com/w/cpp/language/copy_constructor, the compiler will declare a copy constructor for the MappedBuffer class: "If no user-defined copy constructors are provided for a class type (struct, class, or union), the compiler will always declare a copy constructor as a non-explicit inline public member of its class." But that constructor will be deleted: "The implicitly-declared or defaulted copy constructor for class T is defined as deleted if any of the following conditions are true: - ... - T has a user-defined move constructor or move assignment operator (this condition only causes the implicitly-declared, not the defaulted, copy constructor to be deleted)." so MappedBuffer can't currently be copied if I'm not mistaken. Still, this should be made explicit (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-five) so the patch is right. With or without the commit message updated, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/buffer.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/libcamera/internal/buffer.h b/include/libcamera/internal/buffer.h > index b7b0173f93f5..b980deaf58d5 100644 > --- a/include/libcamera/internal/buffer.h > +++ b/include/libcamera/internal/buffer.h > @@ -10,6 +10,7 @@ > #include <sys/mman.h> > #include <vector> > > +#include <libcamera/class.h> > #include <libcamera/buffer.h> > #include <libcamera/span.h> > > @@ -34,6 +35,9 @@ protected: > > int error_; > std::vector<Plane> maps_; > + > +private: > + LIBCAMERA_DISABLE_COPY(MappedBuffer); > }; > > class MappedFrameBuffer : public MappedBuffer
diff --git a/include/libcamera/internal/buffer.h b/include/libcamera/internal/buffer.h index b7b0173f93f5..b980deaf58d5 100644 --- a/include/libcamera/internal/buffer.h +++ b/include/libcamera/internal/buffer.h @@ -10,6 +10,7 @@ #include <sys/mman.h> #include <vector> +#include <libcamera/class.h> #include <libcamera/buffer.h> #include <libcamera/span.h> @@ -34,6 +35,9 @@ protected: int error_; std::vector<Plane> maps_; + +private: + LIBCAMERA_DISABLE_COPY(MappedBuffer); }; class MappedFrameBuffer : public MappedBuffer
Prevent copying and assignment of MappedBuffer types, which could invoke undesired unmapping of mapped memory. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- include/libcamera/internal/buffer.h | 4 ++++ 1 file changed, 4 insertions(+)