[{"id":15106,"web_url":"https://patchwork.libcamera.org/comment/15106/","msgid":"<YCWeI7IZqLVCkC22@pendragon.ideasonboard.com>","date":"2021-02-11T21:14:11","subject":"Re: [libcamera-devel] [PATCH v2 5/6] 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 Thu, Feb 11, 2021 at 01:34:43PM +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> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  include/libcamera/camera.h                      | 6 +++---\n>  include/libcamera/camera_manager.h              | 4 ++--\n>  include/libcamera/controls.h                    | 7 +++----\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>  12 files changed, 36 insertions(+), 30 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index cff9f46e801b..568740a37778 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<Stream *> &streams);\n>  \n> -\tCamera(const Camera &) = delete;\n> -\tCamera &operator=(const Camera &) = delete;\n> -\n>  \tconst std::string &id() const;\n>  \n>  \tSignal<Request *, FrameBuffer *> bufferCompleted;\n> @@ -109,6 +106,9 @@ public:\n>  private:\n>  \tCamera(PipelineHandler *pipe, const std::string &id,\n>  \t       const std::set<Stream *> &streams);\n> +\n> +\tLIBCAMERA_DISABLE_COPY(Camera);\n\nLet's move this right after \"private:\" for consistency.\n\n> +\n>  \t~Camera();\n>  \n>  \tfriend class PipelineHandler;\n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 7b8e533fadd6..467cfd0ac4ac 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<std::shared_ptr<Camera>> 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/controls.h b/include/libcamera/controls.h\n> index 3b7f3347761e..a5c9bda28661 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -14,6 +14,7 @@\n>  #include <unordered_map>\n>  #include <vector>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/geometry.h>\n>  #include <libcamera/span.h>\n>  \n> @@ -220,8 +221,7 @@ public:\n>  \tControlType type() const { return type_; }\n>  \n>  private:\n> -\tControlId &operator=(const ControlId &) = delete;\n> -\tControlId(const ControlId &) = delete;\n> +\tLIBCAMERA_DISABLE_COPY(ControlId);\n\nThis, and Control, should use LIBCAMERA_DISABLE_COPY_AND_MOVE. You can\ndo so in an additional patch, or split this change out to a patch that\nuses the macro and changes the semantics in one go.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  \n>  \tunsigned int id_;\n>  \tstd::string name_;\n> @@ -260,8 +260,7 @@ public:\n>  \t}\n>  \n>  private:\n> -\tControl(const Control &) = delete;\n> -\tControl &operator=(const Control &) = delete;\n> +\tLIBCAMERA_DISABLE_COPY(Control);\n>  };\n>  \n>  class ControlInfo\n> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h\n> index a96aaeae58ce..9e42de280c98 100644\n> --- a/include/libcamera/framebuffer_allocator.h\n> +++ b/include/libcamera/framebuffer_allocator.h\n> @@ -11,6 +11,8 @@\n>  #include <memory>\n>  #include <vector>\n>  \n> +#include <libcamera/class.h>\n> +\n>  namespace libcamera {\n>  \n>  class Camera;\n> @@ -21,9 +23,6 @@ class FrameBufferAllocator\n>  {\n>  public:\n>  \tFrameBufferAllocator(std::shared_ptr<Camera> 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<std::unique_ptr<FrameBuffer>> &buffers(Stream *stream) const;\n>  \n>  private:\n> +\tLIBCAMERA_DISABLE_COPY(FrameBufferAllocator);\n> +\n>  \tstd::shared_ptr<Camera> camera_;\n>  \tstd::map<Stream *, std::vector<std::unique_ptr<FrameBuffer>>> buffers_;\n>  };\n> diff --git a/include/libcamera/internal/byte_stream_buffer.h b/include/libcamera/internal/byte_stream_buffer.h\n> index db59577dc332..ec8b0b2ada86 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 <stdint.h>\n>  #include <type_traits>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/span.h>\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..9d26bd24a321 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -11,6 +11,7 @@\n>  #include <string>\n>  #include <vector>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/controls.h>\n>  #include <libcamera/geometry.h>\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..2f75aaf00dab 100644\n> --- a/include/libcamera/internal/file.h\n> +++ b/include/libcamera/internal/file.h\n> @@ -11,6 +11,7 @@\n>  #include <string>\n>  #include <sys/types.h>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/span.h>\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..5c44d623797a 100644\n> --- a/include/libcamera/internal/log.h\n> +++ b/include/libcamera/internal/log.h\n> @@ -10,6 +10,8 @@\n>  #include <chrono>\n>  #include <sstream>\n>  \n> +#include <libcamera/class.h>\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>  \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..d5913eae88ca 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -15,6 +15,7 @@\n>  #include <sys/types.h>\n>  #include <vector>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/controls.h>\n>  #include <libcamera/object.h>\n>  #include <libcamera/stream.h>\n> @@ -49,8 +50,7 @@ public:\n>  \tstd::unique_ptr<IPAProxy> 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<PipelineHandler>,\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index eb25fa2fd01b..f983ae6302bb 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -11,6 +11,7 @@\n>  #include <string>\n>  #include <vector>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/geometry.h>\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<unsigned int> enumPadCodes(unsigned int pad);\n>  \tstd::vector<SizeRange> 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..f8da608603ef 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -17,6 +17,7 @@\n>  #include <linux/videodev2.h>\n>  \n>  #include <libcamera/buffer.h>\n> +#include <libcamera/class.h>\n>  #include <libcamera/geometry.h>\n>  #include <libcamera/pixel_format.h>\n>  #include <libcamera/signal.h>\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..3354ae9083f1 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -12,6 +12,7 @@\n>  #include <stdint.h>\n>  #include <unordered_set>\n>  \n> +#include <libcamera/class.h>\n>  #include <libcamera/controls.h>\n>  #include <libcamera/signal.h>\n>  \n> @@ -39,8 +40,6 @@ public:\n>  \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\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":"<libcamera-devel-bounces@lists.libcamera.org>","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 8A463BD162\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 11 Feb 2021 21:14:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A2E86374C;\n\tThu, 11 Feb 2021 22:14:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D1C77601B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Feb 2021 22:14:36 +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 3C0E141;\n\tThu, 11 Feb 2021 22:14: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=\"LEsCM5QQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613078076;\n\tbh=UackSW4Lnry8rzQmEWBXJl7TuZhuJ8ChQIRIp3AUZuA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LEsCM5QQmbHMnHECJ0J29cfYLEkEYZ+bRK2NKJA3Vi0pwQV1wAJHx0WVXSHiOppLv\n\t5k+Vo+kKImz6a0Wx9v+Xu/jl7oWBguvTKxeWh+QsH4ovSvz6e9B/C6w97mw5zmT+xj\n\tKn/3HrK7U1Y32IQLfSuj6ZtkacEixtCAIWDUpZFM=","Date":"Thu, 11 Feb 2021 23:14:11 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YCWeI7IZqLVCkC22@pendragon.ideasonboard.com>","References":"<20210211133444.764808-1-kieran.bingham@ideasonboard.com>\n\t<20210211133444.764808-6-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210211133444.764808-6-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 5/6] libcamera: Utilise\n\tLIBCAMERA_DISABLE_COPY","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]