[libcamera-devel,v4,3/6] libcamera: ipa_manager: Simplify addDir() usage

Message ID 20200221163130.4791-4-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • Support loading IPAs from the build tree
Related show

Commit Message

Kieran Bingham Feb. 21, 2020, 4:31 p.m. UTC
The addDir call only returns an error if it can't open the directory.
Callers only care about the number of modules added, and discard any
error information.

Simplify the return value and calling code by returning an unsigned int
of the number of modules loaded.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/include/ipa_manager.h |  2 +-
 src/libcamera/ipa_manager.cpp       | 16 +++++-----------
 2 files changed, 6 insertions(+), 12 deletions(-)

Comments

Laurent Pinchart Feb. 22, 2020, 10:28 a.m. UTC | #1
Hi Kieran,

Thank you for the patch.

On Fri, Feb 21, 2020 at 04:31:27PM +0000, Kieran Bingham wrote:
> The addDir call only returns an error if it can't open the directory.
> Callers only care about the number of modules added, and discard any
> error information.
> 
> Simplify the return value and calling code by returning an unsigned int
> of the number of modules loaded.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  src/libcamera/include/ipa_manager.h |  2 +-
>  src/libcamera/ipa_manager.cpp       | 16 +++++-----------
>  2 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/ipa_manager.h
> index 126f8babbc8f..f13a93d74498 100644
> --- a/src/libcamera/include/ipa_manager.h
> +++ b/src/libcamera/include/ipa_manager.h
> @@ -32,7 +32,7 @@ private:
>  	IPAManager();
>  	~IPAManager();
>  
> -	int addDir(const char *libDir);
> +	unsigned int addDir(const char *libDir);
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 7f0a5d58749b..90dd30030dcb 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -96,7 +96,6 @@ LOG_DEFINE_CATEGORY(IPAManager)
>  IPAManager::IPAManager()
>  {
>  	unsigned int ipaCount = 0;
> -	int ret;
>  
>  	/* User-specified paths take precedence. */
>  	const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
> @@ -105,9 +104,7 @@ IPAManager::IPAManager()
>  			if (dir.empty())
>  				continue;
>  
> -			int ret = addDir(dir.c_str());
> -			if (ret > 0)
> -				ipaCount += ret;
> +			ipaCount += addDir(dir.c_str());
>  		}
>  
>  		if (!ipaCount)
> @@ -116,9 +113,7 @@ IPAManager::IPAManager()
>  	}
>  
>  	/* Load IPAs from the installed system path. */
> -	ret = addDir(IPA_MODULE_DIR);
> -	if (ret > 0)
> -		ipaCount += ret;
> +	ipaCount += addDir(IPA_MODULE_DIR);
>  
>  	if (!ipaCount)
>  		LOG(IPAManager, Warning)
> @@ -153,17 +148,16 @@ IPAManager *IPAManager::instance()
>   * This method tries to create an IPAModule instance for every shared object
>   * found in \a libDir, and skips invalid IPA modules.
>   *
> - * \return Number of modules loaded by this call, or a negative error code
> - * otherwise
> + * \return Number of modules loaded by this call
>   */
> -int IPAManager::addDir(const char *libDir)
> +unsigned int IPAManager::addDir(const char *libDir)
>  {
>  	struct dirent *ent;
>  	DIR *dir;
>  
>  	dir = opendir(libDir);
>  	if (!dir)
> -		return -errno;
> +		return 0;
>  
>  	std::vector<std::string> paths;
>  	while ((ent = readdir(dir)) != nullptr) {

Patch

diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/ipa_manager.h
index 126f8babbc8f..f13a93d74498 100644
--- a/src/libcamera/include/ipa_manager.h
+++ b/src/libcamera/include/ipa_manager.h
@@ -32,7 +32,7 @@  private:
 	IPAManager();
 	~IPAManager();
 
-	int addDir(const char *libDir);
+	unsigned int addDir(const char *libDir);
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index 7f0a5d58749b..90dd30030dcb 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -96,7 +96,6 @@  LOG_DEFINE_CATEGORY(IPAManager)
 IPAManager::IPAManager()
 {
 	unsigned int ipaCount = 0;
-	int ret;
 
 	/* User-specified paths take precedence. */
 	const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
@@ -105,9 +104,7 @@  IPAManager::IPAManager()
 			if (dir.empty())
 				continue;
 
-			int ret = addDir(dir.c_str());
-			if (ret > 0)
-				ipaCount += ret;
+			ipaCount += addDir(dir.c_str());
 		}
 
 		if (!ipaCount)
@@ -116,9 +113,7 @@  IPAManager::IPAManager()
 	}
 
 	/* Load IPAs from the installed system path. */
-	ret = addDir(IPA_MODULE_DIR);
-	if (ret > 0)
-		ipaCount += ret;
+	ipaCount += addDir(IPA_MODULE_DIR);
 
 	if (!ipaCount)
 		LOG(IPAManager, Warning)
@@ -153,17 +148,16 @@  IPAManager *IPAManager::instance()
  * This method tries to create an IPAModule instance for every shared object
  * found in \a libDir, and skips invalid IPA modules.
  *
- * \return Number of modules loaded by this call, or a negative error code
- * otherwise
+ * \return Number of modules loaded by this call
  */
-int IPAManager::addDir(const char *libDir)
+unsigned int IPAManager::addDir(const char *libDir)
 {
 	struct dirent *ent;
 	DIR *dir;
 
 	dir = opendir(libDir);
 	if (!dir)
-		return -errno;
+		return 0;
 
 	std::vector<std::string> paths;
 	while ((ent = readdir(dir)) != nullptr) {