[libcamera-devel] [PATCH 2/3] libcamera: camera: Exclude streams whose configuration cannot be updated

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Tue, Apr 14, 2020 at 07:06:58AM +0000, Umang Jain wrote:
> This prevents a null-deference below in the loop.
> 
> Pointed out by Coverity DefectId=279069
> 
> Signed-off-by: Umang Jain <email at uajain.com>
> ---
>  src/libcamera/camera.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 8c3bb2c..926e414 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -777,9 +777,11 @@ int Camera::configure(CameraConfiguration *config)
>  	p_->activeStreams_.clear();
>  	for (const StreamConfiguration &cfg : *config) {
>  		Stream *stream = cfg.stream();
> -		if (!stream)
> +		if (!stream) {
>  			LOG(Camera, Fatal)
>  				<< "Pipeline handler failed to update stream configuration";
> +			continue;

Note that LOG(..., Fatal) calls std::abort(), so the program will
terminate. We are however considering only doing so in debug builds, so
fixing this issue is a good idea. However, I don't think such failure
should be ignored, the function should instead return an error
immediately. There may be a few things to cleanup before returning
though, to try and leave the Camera object in a valid state.

> +		}
>  
>  		stream->configuration_ = cfg;
>  		p_->activeStreams_.insert(stream);

-- 
Regards,

Laurent Pinchart