[libcamera-devel] [PATCH v2] libcamera: ipa_manager: Fix handling of unset LIBCAMERA_IPA_MODULE_PATH

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Jun 10, 2019 at 11:00:23PM +0200, Niklas Söderlund wrote:
> If the environment variable LIBCAMERA_IPA_MODULE_PATH is not set
> utils::secure_getenv() will return a nullptr. Assigning a nullptr to a
> std::string is not valid and results in a crash,
> 
>     terminate called after throwing an instance of 'std::logic_error'
>       what():  basic_string::_M_construct null not valid
> 
> Fix this by operating directly on the returned char array instead of
> turning it into a std::string.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/ipa_manager.cpp | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index f689aa69b7284092..b9b772d1d5e7795f 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -34,19 +34,22 @@ IPAManager::IPAManager()
>  {
>  	addDir(IPA_MODULE_DIR);
>  
> -	std::string modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
> -	if (modulePaths.empty())
> +	const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
> +	if (!modulePaths)
>  		return;
>  
> -	for (size_t pos = 0, delim = 0; delim != std::string::npos;
> -	     pos = delim + 1) {
> -		delim = modulePaths.find(':', pos);
> -		size_t count = delim == std::string::npos ? delim : delim - pos;
> -		std::string path = modulePaths.substr(pos, count);
> -		if (path.empty())
> -			continue;
> +	while (1) {
> +		const char *delim = strchrnul(modulePaths, ':');
> +		size_t count = delim - modulePaths;
>  
> -		addDir(path.c_str());
> +		std::string path(modulePaths, count);
> +		if (!path.empty())
> +			addDir(path.c_str());

Small improvement, to avoid constructing an empty string,

		if (count) {
			std::string path(modulePaths, count);
			addDir(path.c_str());
		}

Could you test this with a LIBCAMERA_IPA_MODULE_PATH that contains "::"
and that ends with ":" ?

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

> +
> +		if (*delim == '\0')
> +			break;
> +
> +		modulePaths += count + 1;
>  	}
>  }
>  

-- 
Regards,

Laurent Pinchart