[libcamera-devel] [PATCH 7/8] libcamera: camera: ensure streams belong to camera

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tue, Feb 26, 2019 at 03:18:56AM +0100, Niklas Söderlund wrote:
> Before calling into the pipeline handler make sure the streams provided
> by the application actually belongs to the camera.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/camera.cpp | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 938f21fe80ef8ceb..ba8638009992170f 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -253,6 +253,10 @@ Camera::streamConfiguration(std::set<Stream *> &streams)
>  	if (!stateIsAtleast(Free) || !streams.size())
>  		return std::map<Stream *, StreamConfiguration>{};
>  
> +	for (Stream *stream : streams)
> +		if (streams_.find(stream) == streams_.end())
> +			return std::map<Stream *, StreamConfiguration>{};
> +

Could we have braces for the for statement ?

I'm beginning to wonder if the function shouldn't return an error value
to differentiate between the differents errors, and take the std::map as
an output parameter.

I also wonder if this check is worth it. Do you foresee this problem as
common, or even possible without the application going through great
efforts to make this happen ?

>  	return pipe_->streamConfiguration(this, streams);
>  }
>  
> @@ -290,6 +294,10 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
>  		return -EINVAL;
>  	}
>  
> +	for (auto const &iter : config)
> +		if (streams_.find(iter.first) == streams_.end())
> +			return -EINVAL;
> +

Braces here too please.

>  	ret = pipe_->configureStreams(this, config);
>  	if (ret)
>  		return ret;

-- 
Regards,

Laurent Pinchart