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

Kieran Bingham kieran.bingham at ideasonboard.com
Fri Oct 23 10:31:09 CEST 2020


On 23/10/2020 05:37, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> 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.

--
Kieran


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


More information about the libcamera-devel mailing list