From patchwork Mon Oct 3 21:21:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17517 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1971ABD16B for ; Mon, 3 Oct 2022 21:21:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C42F860A82; Mon, 3 Oct 2022 23:21:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1664832103; bh=k/1FbOie6DtkP1kevsnc+h35qGdJPDCcnLW792JXDGo=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=DAmzqw7ErDYeUdHFz9iVPFPHA3WTBEnuAG+LL/C3sfl2mbLh11gfUbHXD2eJNjfyZ u7kVbToWJ9R9JXmMMCc/WrxUVagg5Eefn0h00pj3nPrXGJ7T6RwiXDCQ/ws7vaIwZT 8TdDCW/SRfABsW25ybedz/j2e1mf1DGQ8ZK0Vtn6wYhr+4vbviNUEb7J+Q3jEB9M/U RihcHKj61mM8gi+pthAXZmxn6jiU5uPUZ/jsOs4Nq6XJs/egynKsHu+laCPYkr+DkL lJ+zhbkyR8tF8TZpCAkxnMBKTuQI18NejDER101D7iq98c25fiydBI8+P/ig469zp2 OasAxHuy1shsA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 239C660A7C for ; Mon, 3 Oct 2022 23:21:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TCgBl5/m"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A50BA519 for ; Mon, 3 Oct 2022 23:21:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1664832101; bh=k/1FbOie6DtkP1kevsnc+h35qGdJPDCcnLW792JXDGo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=TCgBl5/m15Y/rX1NlzcO+EIBMOGbuYVH6/MGYuLyhCSExSmggPG8uoLqqcfLNpKS/ mGYzwNJNODu20J02Pl64dtkMlMpY1HlKVfe83uNlOLfTpH8ynNPhyNKKGdakA16QQd +VubniA0jeQMBnIDz0FTpW9iroV/m/7JUkcLYSBs= To: libcamera-devel@lists.libcamera.org Date: Tue, 4 Oct 2022 00:21:27 +0300 Message-Id: <20221003212128.32429-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221003212128.32429-1-laurent.pinchart@ideasonboard.com> References: <20221003212128.32429-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 7/8] libcamera: pipeline_handler: Return unique_ptr from createInstance X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Avoid naked pointer with memory allocation by returning a unique_ptr from PipelineHandlerFactory::createInstance(), in order to increase memory allocation safety. This allows iterating over factories in the CameraManager and unit tests using const pointers. Signed-off-by: Laurent Pinchart Reviewed-by: Xavier Roumegue Reviewed-by: Jacopo Mondi --- include/libcamera/internal/pipeline_handler.h | 8 +++++--- src/libcamera/camera_manager.cpp | 4 ++-- src/libcamera/pipeline_handler.cpp | 8 ++++---- test/ipa/ipa_interface_test.cpp | 4 ++-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index ebbdf2aa391f..ad74dc823290 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -113,7 +113,8 @@ public: private: static void registerType(PipelineHandlerFactory *factory); - virtual PipelineHandler *createInstance(CameraManager *manager) const = 0; + virtual std::unique_ptr + createInstance(CameraManager *manager) const = 0; std::string name_; }; @@ -125,9 +126,10 @@ public: \ handler##Factory() : PipelineHandlerFactory(#handler) {} \ \ private: \ - PipelineHandler *createInstance(CameraManager *manager) const \ + std::unique_ptr \ + createInstance(CameraManager *manager) const \ { \ - return new handler(manager); \ + return std::make_unique(manager); \ } \ }; \ static handler##Factory global_##handler##Factory; diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 7ff4bc6fd019..2c3f2d86c469 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -142,10 +142,10 @@ void CameraManager::Private::createPipelineHandlers() * file and only fallback on all handlers if there is no * configuration file. */ - std::vector &factories = + const std::vector &factories = PipelineHandlerFactory::factories(); - for (PipelineHandlerFactory *factory : factories) { + for (const PipelineHandlerFactory *factory : factories) { LOG(Camera, Debug) << "Found registered pipeline handler '" << factory->name() << "'"; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4470e9041e8e..488dd8d32cdf 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -678,9 +678,9 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name) */ std::shared_ptr PipelineHandlerFactory::create(CameraManager *manager) const { - PipelineHandler *handler = createInstance(manager); + std::unique_ptr handler = createInstance(manager); handler->name_ = name_.c_str(); - return std::shared_ptr(handler); + return std::shared_ptr(std::move(handler)); } /** @@ -727,8 +727,8 @@ std::vector &PipelineHandlerFactory::factories() * macro. It creates a pipeline handler instance associated with the camera * \a manager. * - * \return a pointer to a newly constructed instance of the PipelineHandler - * subclass corresponding to the factory + * \return A unique pointer to a newly constructed instance of the + * PipelineHandler subclass corresponding to the factory */ /** diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 3c0df843ea61..a629abc28da7 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -52,9 +52,9 @@ protected: ipaManager_ = make_unique(); /* Create a pipeline handler for vimc. */ - std::vector &factories = + const std::vector &factories = PipelineHandlerFactory::factories(); - for (PipelineHandlerFactory *factory : factories) { + for (const PipelineHandlerFactory *factory : factories) { if (factory->name() == "PipelineHandlerVimc") { pipe_ = factory->create(nullptr); break;