[libcamera-devel] [PATCH v2 6/6] libcamera: MappedBuffer: Disable copy and assignment

[Laurent Pinchart]

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 at 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 at 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

-- 
Regards,

Laurent Pinchart