[PATCH v1] libcamera: ipa_manager: Store `IPAModule`s in `std::unique_ptr`

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Mon, Mar 03, 2025 at 08:33:35PM +0100, Barnabás Pőcze wrote:
> Express the ownership more clearly by using a smart pointer type.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  include/libcamera/internal/ipa_manager.h |  2 +-
>  src/libcamera/ipa_manager.cpp            | 18 ++++++------------
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> index 16dede0c5..e82c25a64 100644
> --- a/include/libcamera/internal/ipa_manager.h
> +++ b/include/libcamera/internal/ipa_manager.h
> @@ -67,7 +67,7 @@ private:

#include <memory>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  
>  	bool isSignatureValid(IPAModule *ipa) const;
>  
> -	std::vector<IPAModule *> modules_;
> +	std::vector<std::unique_ptr<IPAModule>> modules_;
>  
>  #if HAVE_IPA_PUBKEY
>  	static const uint8_t publicKeyData_[];
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index cfc24d389..830750dcc 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -149,11 +149,7 @@ IPAManager::IPAManager()
>  			<< "No IPA found in '" IPA_MODULE_DIR "'";
>  }
>  
> -IPAManager::~IPAManager()
> -{
> -	for (IPAModule *module : modules_)
> -		delete module;
> -}
> +IPAManager::~IPAManager() = default;
>  
>  /**
>   * \brief Identify shared library objects within a directory
> @@ -226,15 +222,13 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)
>  
>  	unsigned int count = 0;
>  	for (const std::string &file : files) {
> -		IPAModule *ipaModule = new IPAModule(file);
> -		if (!ipaModule->isValid()) {
> -			delete ipaModule;
> +		auto ipaModule = std::make_unique<IPAModule>(file);
> +		if (!ipaModule->isValid())
>  			continue;
> -		}
>  
>  		LOG(IPAManager, Debug) << "Loaded IPA module '" << file << "'";
>  
> -		modules_.push_back(ipaModule);
> +		modules_.push_back(std::move(ipaModule));
>  		count++;
>  	}
>  
> @@ -250,9 +244,9 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)
>  IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,
>  			      uint32_t maxVersion)
>  {
> -	for (IPAModule *module : modules_) {
> +	for (const auto &module : modules_) {
>  		if (module->match(pipe, minVersion, maxVersion))
> -			return module;
> +			return module.get();
>  	}
>  
>  	return nullptr;

-- 
Regards,

Laurent Pinchart