[{"id":15144,"web_url":"https://patchwork.libcamera.org/comment/15144/","msgid":"","date":"2021-02-12T14:03:11","subject":"Re: [libcamera-devel] [PATCH v3 6/7] libcamera: Utilise\n\tLIBCAMERA_DISABLE_COPY","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Fri, Feb 12, 2021 at 01:30:55PM +0000, Kieran Bingham wrote:\n> Replace existing use cases where the copy constructor and copy\n> assignment operator are deleted with the LIBCAMERA_DISABLE_COPY\n> statement\n> \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/camera.h | 5 ++---\n> include/libcamera/camera_manager.h | 4 ++--\n> include/libcamera/framebuffer_allocator.h | 7 ++++---\n> include/libcamera/internal/byte_stream_buffer.h | 4 ++--\n> include/libcamera/internal/camera_sensor.h | 6 +++---\n> include/libcamera/internal/file.h | 6 +++---\n> include/libcamera/internal/log.h | 6 +++++-\n> include/libcamera/internal/pipeline_handler.h | 4 ++--\n> include/libcamera/internal/v4l2_subdevice.h | 5 +++--\n> include/libcamera/internal/v4l2_videodevice.h | 6 +++---\n> include/libcamera/request.h | 5 +++--\n> 11 files changed, 32 insertions(+), 26 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index cff9f46e801b..bd81fb54502e 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -81,9 +81,6 @@ public:\n> \t\t\t\t\t const std::string &id,\n> \t\t\t\t\t const std::set &streams);\n> \n> -\tCamera(const Camera &) = delete;\n> -\tCamera &operator=(const Camera &) = delete;\n> -\n> \tconst std::string &id() const;\n> \n> \tSignal bufferCompleted;\n> @@ -107,6 +104,8 @@ public:\n> \tint stop();\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(Camera)\n> +\n> \tCamera(PipelineHandler *pipe, const std::string &id,\n> \t const std::set &streams);\n> \t~Camera();\n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 7b8e533fadd6..35a59f0df4ca 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -25,8 +25,6 @@ class CameraManager : public Object, public Extensible\n> \tLIBCAMERA_DECLARE_PRIVATE(CameraManager)\n> public:\n> \tCameraManager();\n> -\tCameraManager(const CameraManager &) = delete;\n> -\tCameraManager &operator=(const CameraManager &) = delete;\n> \t~CameraManager();\n> \n> \tint start();\n> @@ -46,6 +44,8 @@ public:\n> \tSignal> cameraRemoved;\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(CameraManager)\n> +\n> \tstatic const std::string version_;\n> \tstatic CameraManager *self_;\n> };\n> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h\n> index a96aaeae58ce..0c85631a1da2 100644\n> --- a/include/libcamera/framebuffer_allocator.h\n> +++ b/include/libcamera/framebuffer_allocator.h\n> @@ -11,6 +11,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> namespace libcamera {\n> \n> class Camera;\n> @@ -21,9 +23,6 @@ class FrameBufferAllocator\n> {\n> public:\n> \tFrameBufferAllocator(std::shared_ptr camera);\n> -\tFrameBufferAllocator(const FrameBufferAllocator &) = delete;\n> -\tFrameBufferAllocator &operator=(const FrameBufferAllocator &) = delete;\n> -\n> \t~FrameBufferAllocator();\n> \n> \tint allocate(Stream *stream);\n> @@ -33,6 +32,8 @@ public:\n> \tconst std::vector> &buffers(Stream *stream) const;\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(FrameBufferAllocator)\n> +\n> \tstd::shared_ptr camera_;\n> \tstd::map>> buffers_;\n> };\n> diff --git a/include/libcamera/internal/byte_stream_buffer.h b/include/libcamera/internal/byte_stream_buffer.h\n> index db59577dc332..866cb9b0b2a6 100644\n> --- a/include/libcamera/internal/byte_stream_buffer.h\n> +++ b/include/libcamera/internal/byte_stream_buffer.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -65,8 +66,7 @@ public:\n> \t}\n> \n> private:\n> -\tByteStreamBuffer(const ByteStreamBuffer &other) = delete;\n> -\tByteStreamBuffer &operator=(const ByteStreamBuffer &other) = delete;\n> +\tLIBCAMERA_DISABLE_COPY(ByteStreamBuffer)\n> \n> \tvoid setOverflow();\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index c8f81882a958..f22ffbfe9f97 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> \n> @@ -45,9 +46,6 @@ public:\n> \texplicit CameraSensor(const MediaEntity *entity);\n> \t~CameraSensor();\n> \n> -\tCameraSensor(const CameraSensor &) = delete;\n> -\tCameraSensor &operator=(const CameraSensor &) = delete;\n> -\n> \tint init();\n> \n> \tconst std::string &model() const { return model_; }\n> @@ -74,6 +72,8 @@ protected:\n> \tstd::string logPrefix() const override;\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(CameraSensor)\n> +\n> \tint generateId();\n> \tint validateSensorDriver();\n> \tvoid initVimcDefaultProperties();\n> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n> index 9b6d011f5e9d..f0b313a5faae 100644\n> --- a/include/libcamera/internal/file.h\n> +++ b/include/libcamera/internal/file.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -34,9 +35,6 @@ public:\n> \tFile();\n> \t~File();\n> \n> -\tFile(const File &) = delete;\n> -\tFile &operator=(const File &) = delete;\n> -\n> \tconst std::string &fileName() const { return name_; }\n> \tvoid setFileName(const std::string &name);\n> \tbool exists() const;\n> @@ -62,6 +60,8 @@ public:\n> \tstatic bool exists(const std::string &name);\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(File)\n> +\n> \tvoid unmapAll();\n> \n> \tstd::string name_;\n> diff --git a/include/libcamera/internal/log.h b/include/libcamera/internal/log.h\n> index 4b10087a4718..e76ae913c857 100644\n> --- a/include/libcamera/internal/log.h\n> +++ b/include/libcamera/internal/log.h\n> @@ -10,6 +10,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/utils.h\"\n> \n> namespace libcamera {\n> @@ -57,7 +59,7 @@ public:\n> \t\t LogSeverity severity);\n> \tLogMessage(const char *fileName, unsigned int line,\n> \t\t const LogCategory &category, LogSeverity severity);\n> -\tLogMessage(const LogMessage &) = delete;\n> +\n\nMaybe this blank line isn't needed ? Apart from that, the patch looks\ngood to me.\n\n> \tLogMessage(LogMessage &&);\n> \t~LogMessage();\n> \n> @@ -70,6 +72,8 @@ public:\n> \tconst std::string msg() const { return msgStream_.str(); }\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(LogMessage)\n> +\n> \tvoid init(const char *fileName, unsigned int line);\n> \n> \tstd::ostringstream msgStream_;\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 0748f8634608..d81c9b85f107 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -49,8 +50,7 @@ public:\n> \tstd::unique_ptr ipa_;\n> \n> private:\n> -\tCameraData(const CameraData &) = delete;\n> -\tCameraData &operator=(const CameraData &) = delete;\n> +\tLIBCAMERA_DISABLE_COPY(CameraData)\n> };\n> \n> class PipelineHandler : public std::enable_shared_from_this,\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index eb25fa2fd01b..d2b9ca55439e 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \"libcamera/internal/formats.h\"\n> @@ -41,8 +42,6 @@ public:\n> \t};\n> \n> \texplicit V4L2Subdevice(const MediaEntity *entity);\n> -\tV4L2Subdevice(const V4L2Subdevice &) = delete;\n> -\tV4L2Subdevice &operator=(const V4L2Subdevice &) = delete;\n> \t~V4L2Subdevice();\n> \n> \tint open();\n> @@ -68,6 +67,8 @@ protected:\n> \tstd::string logPrefix() const override;\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(V4L2Subdevice)\n> +\n> \tstd::vector enumPadCodes(unsigned int pad);\n> \tstd::vector enumPadSizes(unsigned int pad,\n> \t\t\t\t\t unsigned int code);\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 626dfbcd6113..7938343bba8d 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -17,6 +17,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -175,11 +176,8 @@ public:\n> \n> \texplicit V4L2VideoDevice(const std::string &deviceNode);\n> \texplicit V4L2VideoDevice(const MediaEntity *entity);\n> -\tV4L2VideoDevice(const V4L2VideoDevice &) = delete;\n> \t~V4L2VideoDevice();\n> \n> -\tV4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete;\n> -\n> \tint open();\n> \tint open(int handle, enum v4l2_buf_type type);\n> \tvoid close();\n> @@ -219,6 +217,8 @@ protected:\n> \tstd::string logPrefix() const override;\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(V4L2VideoDevice)\n> +\n> \tint getFormatMeta(V4L2DeviceFormat *format);\n> \tint trySetFormatMeta(V4L2DeviceFormat *format, bool set);\n> \n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 655b1324bae8..6e5aad5f6b75 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> \n> @@ -39,8 +40,6 @@ public:\n> \tusing BufferMap = std::map;\n> \n> \tRequest(Camera *camera, uint64_t cookie = 0);\n> -\tRequest(const Request &) = delete;\n> -\tRequest &operator=(const Request &) = delete;\n> \t~Request();\n> \n> \tvoid reuse(ReuseFlag flags = Default);\n> @@ -57,6 +56,8 @@ public:\n> \tbool hasPendingBuffers() const { return !pending_.empty(); }\n> \n> private:\n> +\tLIBCAMERA_DISABLE_COPY(Request)\n> +\n> \tfriend class PipelineHandler;\n> \n> \tvoid complete();","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0A03BBD162\n\tfor ;\n\tFri, 12 Feb 2021 14:03:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D5C763786;\n\tFri, 12 Feb 2021 15:03:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 44C4163781\n\tfor ;\n\tFri, 12 Feb 2021 15:03:37 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B816E8B5;\n\tFri, 12 Feb 2021 15:03:36 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jcnsZRfu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613138616;\n\tbh=K6qtJKRdIcl6NMEF+wZ1NqRgphqh8wc2FrBwu997P4g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jcnsZRfujHEbVEwrzjaLfmp7V7EEhn4i7P9HCnTA9KFem+mcxkgwTyS8nVRr5yRMU\n\tioFWeYBQjsHt6VUKACV9kbZNyssWoAyxGwhdnT8KUwW2Db6LLfP/0mcrRK3lOT9dGL\n\tvZHmLgq+ePUv5/9etzJ2JGS6PoIo7NUPcFTjLzME=","Date":"Fri, 12 Feb 2021 16:03:11 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210212133056.873230-1-kieran.bingham@ideasonboard.com>\n\t<20210212133056.873230-7-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210212133056.873230-7-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 6/7] libcamera: Utilise\n\tLIBCAMERA_DISABLE_COPY","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]