[RFC PATCH v2 06/13] libcamera: simple: Protect against null maxPipeConfig

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sun Jan 26 22:25:29 CET 2025


On Fri, Jan 24, 2025 at 10:57:57PM +0100, Milan Zamazal wrote:
> SimpleCameraData::pipeConfig_ is set to the determined maxPipeConfig if
> no better configuration is found.  In the current code, maxPipeConfig
> should be always set.  But it may be easy to miss that requirement and
> end up with null maxPipeConfig on contingent code changes.
> 
> Let's add a check for nullptr to prevent segmentation fault surprises.
> It doesn't harm.

But that should never happen. I'm fine hardening the code, but not with
an error check like this. If you think the code is too bug-prone, it
should be refactored to make it less bug-prone.

> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index e6bbff5d..300ebbc0 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1078,8 +1078,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  	}
>  
>  	/* If no configuration was large enough, select the largest one. */
> -	if (!pipeConfig_)
> +	if (!pipeConfig_) {
> +		if (!maxPipeConfig) {
> +			LOG(SimplePipeline, Error) << "No valid configuration found";
> +			return Invalid;
> +		}
>  		pipeConfig_ = maxPipeConfig;
> +	}
>  
>  	LOG(SimplePipeline, Debug)
>  		<< "Picked "

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list