From ba3a1adc13b9a2c2e92f438bcf5320d77bd08bd9 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 3 Oct 2022 22:55:11 +0300 Subject: libcamera: pipeline_handler: Return unique_ptr from createInstance 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 ebbdf2aa..ad74dc82 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 7ff4bc6f..2c3f2d86 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 4470e904..488dd8d3 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 3c0df843..a629abc2 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; -- cgit v1.2.1