[libcamera-devel] [PATCH 1/2] libcamera: camera: Ensure queued requests are prepared

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Mon, Jan 30, 2023 at 06:02:43PM +0000, Kieran Bingham via libcamera-devel wrote:
> Invalid, or not correctly reset requests can cause undefined behaviour
> in the pipeline handlers due to unexpected request state.
> 
> If the status has not been reset to Request::RequestPending, it is
> either not new, or has not been correctly procesed through

s/procesed/processed/

or maybe s/procesed/reset/

> Request::reuse().
> 
> This can be caught early by validating the status of the request when it
> is queued to a camera.
> 
> Reject invalid requests before processing them in the pipeline handlers.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/camera.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 0da167a7bc51..48bf19b28979 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -1126,6 +1126,11 @@ int Camera::queueRequest(Request *request)
>  		return -EXDEV;
>  	}
>  
> +	if (request->status() != Request::RequestPending) {
> +		LOG(Camera, Error) << request->toString() << " is not prepared";

"prepared" is the wrong word here. We have a prepared concept for
requests (see Request::Private::prepare()), and it's not related to
this. The same comment applies to the commit message.

Other than this, the patch looks good to me.

> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * The camera state may change until the end of the function. No locking
>  	 * is however needed as PipelineHandler::queueRequest() will handle

-- 
Regards,

Laurent Pinchart