[libcamera-devel] [PATCH] test: controls: control_list: Add status check

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Nov 22, 2019 at 03:03:46PM +0100, Jacopo Mondi wrote:
> Since commit:
> fac471e812a9 ("test: Extract CameraTest class out of camera tests to libtest")
> the control_list is a subclass of CameraTest, and the status returned by
> the base class init() operation should be inspected to avoid accessing
> uninitialized fields during the run() operation execution.

Nitpicking, you can remove the colon and reflow text here:

Since commit fac471e812a9 ("test: Extract CameraTest class out of
camera tests to libtest") the control_list is a subclass of CameraTest,
and the status returned by the base class init() operation should be
inspected to avoid accessing uninitialized fields during the run()
operation execution.

Only the Fixes: line should avoid breaking the commit message text.

> If the VIMC test module is not loaded, executing the test results in a
> segfault. Fix this by adding the init() operation where to status_ flag
> is checked for errors.
> 
> Fixes: fac471e812a9 ("test: Extract CameraTest class out of camera tests to libtest")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Good catch, thanks.

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

> ---
>  test/controls/control_list.cpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index 4d212abd09e6..5374c6f99f80 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -29,7 +29,12 @@ public:
>  	}
>  
>  protected:
> -	int run()
> +	int init() override
> +	{
> +		return status_;
> +	}
> +
> +	int run() override
>  	{
>  		CameraControlValidator validator(camera_.get());
>  		ControlList list(controls::controls, &validator);

-- 
Regards,

Laurent Pinchart