[libcamera-devel] [PATCH 4/5] libcamera: Utilise DELETE_COPY_AND_ASSIGN

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Oct 23 20:27:41 CEST 2020


Hi Kieran,

On Fri, Oct 23, 2020 at 09:31:09AM +0100, Kieran Bingham wrote:
> On 23/10/2020 05:37, Laurent Pinchart wrote:
> > On Thu, Oct 22, 2020 at 02:56:04PM +0100, Kieran Bingham wrote:
> >> Replace existing use cases where the copy constructor and copy assignment operator
> >> are deleted with the DELETE_COPY_AND_ASSIGN statement
> > 
> > Same comments as for patch 3/5 (especially about disabling both copy and
> > move for most classes). There one more small comment below.
> 
> Ok, so I guess that answers my question there.
> 
> However your statement 'for most classes', and not all makes me want to
> do that on top.
> 
> If it's all of them, then it can be squashed in this patch ... but I
> don't want to mix and match conversions, and functional changes.

As replied in a separate e-mail, I'm fine doing it on top (I agree it's
best), but with a bonus point if it was in the same series otherwise
we'll likely forget about it. I can help if needed.

> >> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >> ---
> >>  include/libcamera/camera.h                      | 7 ++++---
> >>  include/libcamera/camera_manager.h              | 5 +++--
> >>  include/libcamera/controls.h                    | 7 +++----
> >>  include/libcamera/framebuffer_allocator.h       | 7 ++++---
> >>  include/libcamera/internal/byte_stream_buffer.h | 4 ++--
> >>  include/libcamera/internal/camera_sensor.h      | 6 +++---
> >>  include/libcamera/internal/file.h               | 6 +++---
> >>  include/libcamera/internal/log.h                | 6 +++++-
> >>  include/libcamera/internal/pipeline_handler.h   | 4 ++--
> >>  include/libcamera/internal/v4l2_subdevice.h     | 5 +++--
> >>  include/libcamera/internal/v4l2_videodevice.h   | 6 +++---
> >>  include/libcamera/request.h                     | 5 +++--
> >>  12 files changed, 38 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> >> index 79ff8d6b67a4..a9b855bb4691 100644
> >> --- a/include/libcamera/camera.h
> >> +++ b/include/libcamera/camera.h
> >> @@ -12,6 +12,7 @@
> >>  #include <stdint.h>
> >>  #include <string>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/controls.h>
> >>  #include <libcamera/object.h>
> >>  #include <libcamera/request.h>
> >> @@ -77,9 +78,6 @@ public:
> >>  					      const std::string &id,
> >>  					      const std::set<Stream *> &streams);
> >>  
> >> -	Camera(const Camera &) = delete;
> >> -	Camera &operator=(const Camera &) = delete;
> >> -
> >>  	const std::string &id() const;
> >>  
> >>  	Signal<Request *, FrameBuffer *> bufferCompleted;
> >> @@ -105,6 +103,9 @@ public:
> >>  private:
> >>  	Camera(PipelineHandler *pipe, const std::string &id,
> >>  	       const std::set<Stream *> &streams);
> >> +
> >> +	DELETE_COPY_AND_ASSIGN(Camera);
> >> +
> >>  	~Camera();
> >>  
> >>  	class Private;
> >> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> >> index 9eb2b6f5a5f5..1442c4701432 100644
> >> --- a/include/libcamera/camera_manager.h
> >> +++ b/include/libcamera/camera_manager.h
> >> @@ -12,6 +12,7 @@
> >>  #include <sys/types.h>
> >>  #include <vector>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/object.h>
> >>  #include <libcamera/signal.h>
> >>  
> >> @@ -24,8 +25,6 @@ class CameraManager : public Object
> >>  {
> >>  public:
> >>  	CameraManager();
> >> -	CameraManager(const CameraManager &) = delete;
> >> -	CameraManager &operator=(const CameraManager &) = delete;
> >>  	~CameraManager();
> >>  
> >>  	int start();
> >> @@ -48,6 +47,8 @@ public:
> >>  	Signal<std::shared_ptr<Camera>> cameraRemoved;
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(CameraManager);
> >> +
> >>  	static const std::string version_;
> >>  	static CameraManager *self_;
> >>  
> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> >> index 80944efc133a..8a84170a0293 100644
> >> --- a/include/libcamera/controls.h
> >> +++ b/include/libcamera/controls.h
> >> @@ -13,6 +13,7 @@
> >>  #include <string>
> >>  #include <unordered_map>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/geometry.h>
> >>  #include <libcamera/span.h>
> >>  
> >> @@ -218,8 +219,7 @@ public:
> >>  	ControlType type() const { return type_; }
> >>  
> >>  private:
> >> -	ControlId &operator=(const ControlId &) = delete;
> >> -	ControlId(const ControlId &) = delete;
> >> +	DELETE_COPY_AND_ASSIGN(ControlId);
> >>  
> >>  	unsigned int id_;
> >>  	std::string name_;
> >> @@ -258,8 +258,7 @@ public:
> >>  	}
> >>  
> >>  private:
> >> -	Control(const Control &) = delete;
> >> -	Control &operator=(const Control &) = delete;
> >> +	DELETE_COPY_AND_ASSIGN(Control);
> >>  };
> >>  
> >>  class ControlInfo
> >> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> >> index a96aaeae58ce..5c540f23f8d9 100644
> >> --- a/include/libcamera/framebuffer_allocator.h
> >> +++ b/include/libcamera/framebuffer_allocator.h
> >> @@ -11,6 +11,8 @@
> >>  #include <memory>
> >>  #include <vector>
> >>  
> >> +#include <libcamera/class.h>
> >> +
> >>  namespace libcamera {
> >>  
> >>  class Camera;
> >> @@ -21,9 +23,6 @@ class FrameBufferAllocator
> >>  {
> >>  public:
> >>  	FrameBufferAllocator(std::shared_ptr<Camera> camera);
> >> -	FrameBufferAllocator(const FrameBufferAllocator &) = delete;
> >> -	FrameBufferAllocator &operator=(const FrameBufferAllocator &) = delete;
> >> -
> >>  	~FrameBufferAllocator();
> >>  
> >>  	int allocate(Stream *stream);
> >> @@ -33,6 +32,8 @@ public:
> >>  	const std::vector<std::unique_ptr<FrameBuffer>> &buffers(Stream *stream) const;
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(FrameBufferAllocator);
> >> +
> >>  	std::shared_ptr<Camera> camera_;
> >>  	std::map<Stream *, std::vector<std::unique_ptr<FrameBuffer>>> buffers_;
> >>  };
> >> diff --git a/include/libcamera/internal/byte_stream_buffer.h b/include/libcamera/internal/byte_stream_buffer.h
> >> index db59577dc332..b38c026c8eeb 100644
> >> --- a/include/libcamera/internal/byte_stream_buffer.h
> >> +++ b/include/libcamera/internal/byte_stream_buffer.h
> >> @@ -11,6 +11,7 @@
> >>  #include <stdint.h>
> >>  #include <type_traits>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/span.h>
> >>  
> >>  namespace libcamera {
> >> @@ -65,8 +66,7 @@ public:
> >>  	}
> >>  
> >>  private:
> >> -	ByteStreamBuffer(const ByteStreamBuffer &other) = delete;
> >> -	ByteStreamBuffer &operator=(const ByteStreamBuffer &other) = delete;
> >> +	DELETE_COPY_AND_ASSIGN(ByteStreamBuffer);
> >>  
> >>  	void setOverflow();
> >>  
> >> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> >> index b9eba89f00fb..f03791847af2 100644
> >> --- a/include/libcamera/internal/camera_sensor.h
> >> +++ b/include/libcamera/internal/camera_sensor.h
> >> @@ -11,6 +11,7 @@
> >>  #include <string>
> >>  #include <vector>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/controls.h>
> >>  #include <libcamera/geometry.h>
> >>  
> >> @@ -41,9 +42,6 @@ public:
> >>  	explicit CameraSensor(const MediaEntity *entity);
> >>  	~CameraSensor();
> >>  
> >> -	CameraSensor(const CameraSensor &) = delete;
> >> -	CameraSensor &operator=(const CameraSensor &) = delete;
> >> -
> >>  	int init();
> >>  
> >>  	const std::string &model() const { return model_; }
> >> @@ -68,6 +66,8 @@ protected:
> >>  	std::string logPrefix() const override;
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(CameraSensor);
> >> +
> >>  	int generateId();
> >>  
> >>  	const MediaEntity *entity_;
> >> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h
> >> index 9b6d011f5e9d..00f8d49d3882 100644
> >> --- a/include/libcamera/internal/file.h
> >> +++ b/include/libcamera/internal/file.h
> >> @@ -11,6 +11,7 @@
> >>  #include <string>
> >>  #include <sys/types.h>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/span.h>
> >>  
> >>  namespace libcamera {
> >> @@ -34,9 +35,6 @@ public:
> >>  	File();
> >>  	~File();
> >>  
> >> -	File(const File &) = delete;
> >> -	File &operator=(const File &) = delete;
> >> -
> >>  	const std::string &fileName() const { return name_; }
> >>  	void setFileName(const std::string &name);
> >>  	bool exists() const;
> >> @@ -62,6 +60,8 @@ public:
> >>  	static bool exists(const std::string &name);
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(File);
> >> +
> >>  	void unmapAll();
> >>  
> >>  	std::string name_;
> >> diff --git a/include/libcamera/internal/log.h b/include/libcamera/internal/log.h
> >> index 4b10087a4718..9e312b6605d3 100644
> >> --- a/include/libcamera/internal/log.h
> >> +++ b/include/libcamera/internal/log.h
> >> @@ -10,6 +10,8 @@
> >>  #include <chrono>
> >>  #include <sstream>
> >>  
> >> +#include <libcamera/class.h>
> >> +
> >>  #include "libcamera/internal/utils.h"
> >>  
> >>  namespace libcamera {
> >> @@ -57,7 +59,7 @@ public:
> >>  		   LogSeverity severity);
> >>  	LogMessage(const char *fileName, unsigned int line,
> >>  		   const LogCategory &category, LogSeverity severity);
> >> -	LogMessage(const LogMessage &) = delete;
> >> +
> > 
> > You can drop this blank line.
> > 
> >>  	LogMessage(LogMessage &&);
> >>  	~LogMessage();
> >>  
> >> @@ -70,6 +72,8 @@ public:
> >>  	const std::string msg() const { return msgStream_.str(); }
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(LogMessage);
> >> +
> >>  	void init(const char *fileName, unsigned int line);
> >>  
> >>  	std::ostringstream msgStream_;
> >> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> >> index a4e1b529c461..11a57e2f63c6 100644
> >> --- a/include/libcamera/internal/pipeline_handler.h
> >> +++ b/include/libcamera/internal/pipeline_handler.h
> >> @@ -15,6 +15,7 @@
> >>  #include <sys/types.h>
> >>  #include <vector>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/controls.h>
> >>  #include <libcamera/object.h>
> >>  #include <libcamera/stream.h>
> >> @@ -50,8 +51,7 @@ public:
> >>  	std::unique_ptr<IPAProxy> ipa_;
> >>  
> >>  private:
> >> -	CameraData(const CameraData &) = delete;
> >> -	CameraData &operator=(const CameraData &) = delete;
> >> +	DELETE_COPY_AND_ASSIGN(CameraData);
> >>  };
> >>  
> >>  class PipelineHandler : public std::enable_shared_from_this<PipelineHandler>,
> >> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> >> index 02ee3e914a8b..28abb9299ce9 100644
> >> --- a/include/libcamera/internal/v4l2_subdevice.h
> >> +++ b/include/libcamera/internal/v4l2_subdevice.h
> >> @@ -10,6 +10,7 @@
> >>  #include <string>
> >>  #include <vector>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/geometry.h>
> >>  
> >>  #include "libcamera/internal/formats.h"
> >> @@ -40,8 +41,6 @@ public:
> >>  	};
> >>  
> >>  	explicit V4L2Subdevice(const MediaEntity *entity);
> >> -	V4L2Subdevice(const V4L2Subdevice &) = delete;
> >> -	V4L2Subdevice &operator=(const V4L2Subdevice &) = delete;
> >>  	~V4L2Subdevice();
> >>  
> >>  	int open();
> >> @@ -67,6 +66,8 @@ protected:
> >>  	std::string logPrefix() const override;
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(V4L2Subdevice);
> >> +
> >>  	std::vector<unsigned int> enumPadCodes(unsigned int pad);
> >>  	std::vector<SizeRange> enumPadSizes(unsigned int pad,
> >>  					    unsigned int code);
> >> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> >> index 53f6a2d5515b..40d6499fce75 100644
> >> --- a/include/libcamera/internal/v4l2_videodevice.h
> >> +++ b/include/libcamera/internal/v4l2_videodevice.h
> >> @@ -16,6 +16,7 @@
> >>  #include <linux/videodev2.h>
> >>  
> >>  #include <libcamera/buffer.h>
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/geometry.h>
> >>  #include <libcamera/pixel_format.h>
> >>  #include <libcamera/signal.h>
> >> @@ -172,11 +173,8 @@ public:
> >>  
> >>  	explicit V4L2VideoDevice(const std::string &deviceNode);
> >>  	explicit V4L2VideoDevice(const MediaEntity *entity);
> >> -	V4L2VideoDevice(const V4L2VideoDevice &) = delete;
> >>  	~V4L2VideoDevice();
> >>  
> >> -	V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete;
> >> -
> >>  	int open();
> >>  	int open(int handle, enum v4l2_buf_type type);
> >>  	void close();
> >> @@ -219,6 +217,8 @@ protected:
> >>  	std::string logPrefix() const override;
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(V4L2VideoDevice);
> >> +
> >>  	int getFormatMeta(V4L2DeviceFormat *format);
> >>  	int trySetFormatMeta(V4L2DeviceFormat *format, bool set);
> >>  
> >> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> >> index 655b1324bae8..4a34ecddc16a 100644
> >> --- a/include/libcamera/request.h
> >> +++ b/include/libcamera/request.h
> >> @@ -12,6 +12,7 @@
> >>  #include <stdint.h>
> >>  #include <unordered_set>
> >>  
> >> +#include <libcamera/class.h>
> >>  #include <libcamera/controls.h>
> >>  #include <libcamera/signal.h>
> >>  
> >> @@ -39,8 +40,6 @@ public:
> >>  	using BufferMap = std::map<const Stream *, FrameBuffer *>;
> >>  
> >>  	Request(Camera *camera, uint64_t cookie = 0);
> >> -	Request(const Request &) = delete;
> >> -	Request &operator=(const Request &) = delete;
> >>  	~Request();
> >>  
> >>  	void reuse(ReuseFlag flags = Default);
> >> @@ -57,6 +56,8 @@ public:
> >>  	bool hasPendingBuffers() const { return !pending_.empty(); }
> >>  
> >>  private:
> >> +	DELETE_COPY_AND_ASSIGN(Request);
> >> +
> >>  	friend class PipelineHandler;
> >>  
> >>  	void complete();

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list