[libcamera-devel,v3] libcamera: ipa_module: add path to module loading error message

1596 diff mbox series

Add an error message to tell, if an IPA module failed to load, the
path to the IPA module shared object that was attempted to be loaded.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes in v3:
- error message wording change
- moved to under the close: label

Changes in v2: don't show module version

 src/libcamera/ipa_module.cpp | 5 +++++
 1 file changed, 5 insertions(+)

Hi Paul,

Thank you for the patch.

On Wed, Jul 03, 2019 at 10:58:25PM +0900, Paul Elder wrote:
> Add an error message to tell, if an IPA module failed to load, the
> path to the IPA module shared object that was attempted to be loaded.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> Changes in v3:
> - error message wording change
> - moved to under the close: label
> 
> Changes in v2: don't show module version
> 
>  src/libcamera/ipa_module.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> index 58e8d1d..477f4b8 100644
> --- a/src/libcamera/ipa_module.cpp
> +++ b/src/libcamera/ipa_module.cpp
> @@ -289,6 +289,11 @@ int IPAModule::loadIPAModuleInfo()
>  unmap:
>  	munmap(map, soSize);
>  close:
> +	if (ret)
> +		LOG(IPAModule, Error)
> +			<< "Error loading IPA module "

I would write "Error loading IPA module info for " as this method loads
the module info only.

> +			<< libPath_;

This can fit on two lines:

		LOG(IPAModule, Error)
			<< "Error loading IPA module info for " << libPath_;

Apart from that,

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

By the way don't forget to remove the history changelog (everything from
--- onwards) before pushing.

> +
>  	close(fd);
>  	return ret;
>  }

Hi Paul,

Thanks for your work.

On 2019-07-03 22:58:25 +0900, Paul Elder wrote:
> Add an error message to tell, if an IPA module failed to load, the
> path to the IPA module shared object that was attempted to be loaded.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

With Laurent's suggestions fixed,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
> Changes in v3:
> - error message wording change
> - moved to under the close: label
> 
> Changes in v2: don't show module version
> 
>  src/libcamera/ipa_module.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> index 58e8d1d..477f4b8 100644
> --- a/src/libcamera/ipa_module.cpp
> +++ b/src/libcamera/ipa_module.cpp
> @@ -289,6 +289,11 @@ int IPAModule::loadIPAModuleInfo()
>  unmap:
>  	munmap(map, soSize);
>  close:
> +	if (ret)
> +		LOG(IPAModule, Error)
> +			<< "Error loading IPA module "
> +			<< libPath_;
> +
>  	close(fd);
>  	return ret;
>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel