Patch Detail
Show a patch.
GET /api/patches/13089/?format=api
{ "id": 13089, "url": "https://patchwork.libcamera.org/api/patches/13089/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13089/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210723040036.32346-4-laurent.pinchart@ideasonboard.com>", "date": "2021-07-23T04:00:22", "name": "[libcamera-devel,RFC,03/17] libcamera: base: class: Don't pass Extensible pointer to Private constructor", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "0d594dfcbcb605033f7865767d90ec5a68d4f239", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/13089/mbox/", "series": [ { "id": 2270, "url": "https://patchwork.libcamera.org/api/series/2270/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2270", "date": "2021-07-23T04:00:19", "name": "libcamera: Replace CameraData with Camera::Private", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2270/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13089/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13089/checks/", "tags": {}, "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 5E4F8C322B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 23 Jul 2021 04:00:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DF70687BB;\n\tFri, 23 Jul 2021 06:00:47 +0200 (CEST)", "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 019F7687A3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 23 Jul 2021 06:00:44 +0200 (CEST)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9AF2B56B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 23 Jul 2021 06:00:43 +0200 (CEST)" ], "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=\"KpEmgCeq\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627012843;\n\tbh=XbfCclM8e+cytzdWbwgdQ40lB1HdldzVI1F/nxcc5bA=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=KpEmgCeqwRMgI7/0p41xS0q6+XSZM1Xr7D/6mBGG6xCDUQ8tB6W5Tj0GYabQWd8Ct\n\t+B9/UTIwYjHkGSdkYehBnVYGkD1k5Z7+sYbG8ALjXGXk1R45skifuItwjPiQ6ThVYy\n\t4eaBwtTFxMVnnFc1QpO5tJSg8Qr1vd971xX6YTm8=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 23 Jul 2021 07:00:22 +0300", "Message-Id": "<20210723040036.32346-4-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>", "References": "<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [RFC PATCH 03/17] libcamera: base: class: Don't\n\tpass Extensible pointer to Private constructor", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The Extensible and Extensible::Private classes contain pointers to each\nother. These pointers are initialized in the respective class's\nconstructor, by passing a pointer to the other class to each\nconstructor. This particular construct reduces the flexibility of the\nExtensible pattern, as the Private class instance has to be allocated\nand constructed in the members initializer list of the Extensible\nclass's constructor. It is thus impossible to perform any operation on\nthe Private class between its construction and the construction of the\nExtensible class, or to subclass the Private class without subclassing\nthe Extensible class.\n\nTo make the design pattern more flexible, don't pass the pointer to the\nExtensible class to the Private class's constructor, but initialize the\npointer manually in the Extensible class's constructor. This requires a\nconst_cast as the o_ member of the Private class is const.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/base/class.h | 3 ++-\n include/libcamera/internal/framebuffer.h | 2 +-\n src/android/camera_hal_config.cpp | 7 +++----\n src/android/mm/cros_camera_buffer.cpp | 4 ++--\n src/android/mm/generic_camera_buffer.cpp | 3 +--\n src/libcamera/base/class.cpp | 6 +++---\n src/libcamera/camera.cpp | 10 +++++-----\n src/libcamera/camera_manager.cpp | 8 ++++----\n src/libcamera/framebuffer.cpp | 6 +++---\n 9 files changed, 24 insertions(+), 25 deletions(-)", "diff": "diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h\nindex 9c7f0f2e6e27..c9d9cd949ca1 100644\n--- a/include/libcamera/base/class.h\n+++ b/include/libcamera/base/class.h\n@@ -64,7 +64,7 @@ public:\n \tclass Private\n \t{\n \tpublic:\n-\t\tPrivate(Extensible *o);\n+\t\tPrivate();\n \t\tvirtual ~Private();\n \n #ifndef __DOXYGEN__\n@@ -82,6 +82,7 @@ public:\n #endif\n \n \tprivate:\n+\t\tfriend class Extensible;\n \t\tExtensible *const o_;\n \t};\n \ndiff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h\nindex a11e895d9b88..8c187adf70c7 100644\n--- a/include/libcamera/internal/framebuffer.h\n+++ b/include/libcamera/internal/framebuffer.h\n@@ -52,7 +52,7 @@ class FrameBuffer::Private : public Extensible::Private\n \tLIBCAMERA_DECLARE_PUBLIC(FrameBuffer)\n \n public:\n-\tPrivate(FrameBuffer *buffer);\n+\tPrivate();\n \n \tvoid setRequest(Request *request) { request_ = request; }\n \ndiff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\nindex 833cf4ba98a9..bbbb1410b52c 100644\n--- a/src/android/camera_hal_config.cpp\n+++ b/src/android/camera_hal_config.cpp\n@@ -32,7 +32,7 @@ class CameraHalConfig::Private : public Extensible::Private\n \tLIBCAMERA_DECLARE_PUBLIC(CameraHalConfig)\n \n public:\n-\tPrivate(CameraHalConfig *halConfig);\n+\tPrivate();\n \n \tint parseConfigFile(FILE *fh, std::map<std::string, CameraConfigData> *cameras);\n \n@@ -50,8 +50,7 @@ private:\n \tstd::map<std::string, CameraConfigData> *cameras_;\n };\n \n-CameraHalConfig::Private::Private(CameraHalConfig *halConfig)\n-\t: Extensible::Private(halConfig)\n+CameraHalConfig::Private::Private()\n {\n }\n \n@@ -344,7 +343,7 @@ int CameraHalConfig::Private::parseConfigFile(FILE *fh,\n }\n \n CameraHalConfig::CameraHalConfig()\n-\t: Extensible(new Private(this)), exists_(false), valid_(false)\n+\t: Extensible(new Private()), exists_(false), valid_(false)\n {\n \tparseConfigurationFile();\n }\ndiff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp\nindex bb55b95e3a39..437502fb8276 100644\n--- a/src/android/mm/cros_camera_buffer.cpp\n+++ b/src/android/mm/cros_camera_buffer.cpp\n@@ -47,8 +47,8 @@ private:\n CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,\n \t\t\t buffer_handle_t camera3Buffer,\n \t\t\t [[maybe_unused]] int flags)\n-\t: Extensible::Private(cameraBuffer), handle_(camera3Buffer),\n-\t numPlanes_(0), valid_(false), registered_(false)\n+\t: handle_(camera3Buffer), numPlanes_(0), valid_(false),\n+\t registered_(false)\n {\n \tbufferManager_ = cros::CameraBufferManager::GetInstance();\n \ndiff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\nindex 166be36efd5b..2a4b77ea5236 100644\n--- a/src/android/mm/generic_camera_buffer.cpp\n+++ b/src/android/mm/generic_camera_buffer.cpp\n@@ -34,9 +34,8 @@ public:\n \tsize_t jpegBufferSize(size_t maxJpegBufferSize) const;\n };\n \n-CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,\n+CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,\n \t\t\t buffer_handle_t camera3Buffer, int flags)\n-\t: Extensible::Private(cameraBuffer)\n {\n \tmaps_.reserve(camera3Buffer->numFds);\n \terror_ = 0;\ndiff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp\nindex d4f0ac64ad48..d24043c20e55 100644\n--- a/src/libcamera/base/class.cpp\n+++ b/src/libcamera/base/class.cpp\n@@ -151,6 +151,7 @@ namespace libcamera {\n Extensible::Extensible(Extensible::Private *d)\n \t: d_(d)\n {\n+\t*const_cast<Extensible **>(&d_->o_) = this;\n }\n \n /**\n@@ -182,10 +183,9 @@ Extensible::Extensible(Extensible::Private *d)\n \n /**\n * \\brief Construct an instance of an Extensible class private data\n- * \\param[in] o Pointer to the public class object\n */\n-Extensible::Private::Private(Extensible *o)\n-\t: o_(o)\n+Extensible::Private::Private()\n+\t: o_(nullptr)\n {\n }\n \ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex c8858e71d105..c126b49290ce 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -344,7 +344,7 @@ public:\n \t\tCameraRunning,\n \t};\n \n-\tPrivate(Camera *camera, PipelineHandler *pipe, const std::string &id,\n+\tPrivate(PipelineHandler *pipe, const std::string &id,\n \t\tconst std::set<Stream *> &streams);\n \t~Private();\n \n@@ -368,11 +368,11 @@ private:\n \tstd::atomic<State> state_;\n };\n \n-Camera::Private::Private(Camera *camera, PipelineHandler *pipe,\n+Camera::Private::Private(PipelineHandler *pipe,\n \t\t\t const std::string &id,\n \t\t\t const std::set<Stream *> &streams)\n-\t: Extensible::Private(camera), pipe_(pipe->shared_from_this()), id_(id),\n-\t streams_(streams), disconnected_(false), state_(CameraAvailable)\n+\t: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),\n+\t disconnected_(false), state_(CameraAvailable)\n {\n }\n \n@@ -632,7 +632,7 @@ const std::string &Camera::id() const\n \n Camera::Camera(PipelineHandler *pipe, const std::string &id,\n \t const std::set<Stream *> &streams)\n-\t: Extensible(new Private(this, pipe, id, streams))\n+\t: Extensible(new Private(pipe, id, streams))\n {\n }\n \ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 1c79308ad4b5..384a574f2baa 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -39,7 +39,7 @@ class CameraManager::Private : public Extensible::Private, public Thread\n \tLIBCAMERA_DECLARE_PUBLIC(CameraManager)\n \n public:\n-\tPrivate(CameraManager *cm);\n+\tPrivate();\n \n \tint start();\n \tvoid addCamera(std::shared_ptr<Camera> camera,\n@@ -74,8 +74,8 @@ private:\n \tProcessManager processManager_;\n };\n \n-CameraManager::Private::Private(CameraManager *cm)\n-\t: Extensible::Private(cm), initialized_(false)\n+CameraManager::Private::Private()\n+\t: initialized_(false)\n {\n }\n \n@@ -258,7 +258,7 @@ void CameraManager::Private::removeCamera(Camera *camera)\n CameraManager *CameraManager::self_ = nullptr;\n \n CameraManager::CameraManager()\n-\t: Extensible(new CameraManager::Private(this))\n+\t: Extensible(new CameraManager::Private())\n {\n \tif (self_)\n \t\tLOG(Camera, Fatal)\ndiff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp\nindex 40bf64b0a4fe..48d14b31f68d 100644\n--- a/src/libcamera/framebuffer.cpp\n+++ b/src/libcamera/framebuffer.cpp\n@@ -100,8 +100,8 @@ LOG_DEFINE_CATEGORY(Buffer)\n * \\brief Array of per-plane metadata\n */\n \n-FrameBuffer::Private::Private(FrameBuffer *buffer)\n-\t: Extensible::Private(buffer), request_(nullptr)\n+FrameBuffer::Private::Private()\n+\t: request_(nullptr)\n {\n }\n \n@@ -176,7 +176,7 @@ FrameBuffer::Private::Private(FrameBuffer *buffer)\n * \\param[in] cookie Cookie\n */\n FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)\n-\t: Extensible(new Private(this)), planes_(planes), cookie_(cookie)\n+\t: Extensible(new Private()), planes_(planes), cookie_(cookie)\n {\n }\n \n", "prefixes": [ "libcamera-devel", "RFC", "03/17" ] }