[libcamera-devel] test: camera: Fix initialisation

1328 diff mbox series

Three tests {capture,configuration_set,statemachine} override the
CameraTest::init() function, and call it as the first action.

However they were not checking the return value, and each of the tests
will segfault if the VIMC camera is not obtained.

Check the return value of the CameraTest base class initialisation and
return any errors to the test suite if initialisation fails.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 test/camera/capture.cpp           | 4 +++-
 test/camera/configuration_set.cpp | 4 +++-
 test/camera/statemachine.cpp      | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

Hi Kieran,

Thank you for the patch.

On Thu, May 30, 2019 at 02:38:49PM +0100, Kieran Bingham wrote:
> Three tests {capture,configuration_set,statemachine} override the
> CameraTest::init() function, and call it as the first action.
> 
> However they were not checking the return value, and each of the tests
> will segfault if the VIMC camera is not obtained.
> 
> Check the return value of the CameraTest base class initialisation and
> return any errors to the test suite if initialisation fails.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  test/camera/capture.cpp           | 4 +++-
>  test/camera/configuration_set.cpp | 4 +++-
>  test/camera/statemachine.cpp      | 4 +++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index c0835c250c65..98e71905531c 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -42,7 +42,9 @@ protected:
>  
>  	int init() override
>  	{
> -		CameraTest::init();
> +		int ret = CameraTest::init();
> +		if (ret)
> +			return ret;
>  
>  		config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!config_ || config_->size() != 1) {
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index 9f10f795a5d8..f88da96ca2b7 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -18,7 +18,9 @@ class ConfigurationSet : public CameraTest
>  protected:
>  	int init() override
>  	{
> -		CameraTest::init();
> +		int ret = CameraTest::init();
> +		if (ret)
> +			return ret;
>  
>  		config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!config_ || config_->size() != 1) {
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index d489f197e402..84d2a6fab5f0 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -235,7 +235,9 @@ protected:
>  
>  	int init() override
>  	{
> -		CameraTest::init();
> +		int ret = CameraTest::init();
> +		if (ret)
> +			return ret;
>  
>  		defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!defconf_) {

Hi Kieran,

Thanks for your work.

On 2019-05-30 14:38:49 +0100, Kieran Bingham wrote:
> Three tests {capture,configuration_set,statemachine} override the
> CameraTest::init() function, and call it as the first action.
> 
> However they were not checking the return value, and each of the tests
> will segfault if the VIMC camera is not obtained.
> 
> Check the return value of the CameraTest base class initialisation and
> return any errors to the test suite if initialisation fails.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  test/camera/capture.cpp           | 4 +++-
>  test/camera/configuration_set.cpp | 4 +++-
>  test/camera/statemachine.cpp      | 4 +++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index c0835c250c65..98e71905531c 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -42,7 +42,9 @@ protected:
>  
>  	int init() override
>  	{
> -		CameraTest::init();
> +		int ret = CameraTest::init();
> +		if (ret)
> +			return ret;
>  
>  		config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!config_ || config_->size() != 1) {
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index 9f10f795a5d8..f88da96ca2b7 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -18,7 +18,9 @@ class ConfigurationSet : public CameraTest
>  protected:
>  	int init() override
>  	{
> -		CameraTest::init();
> +		int ret = CameraTest::init();
> +		if (ret)
> +			return ret;
>  
>  		config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!config_ || config_->size() != 1) {
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index d489f197e402..84d2a6fab5f0 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -235,7 +235,9 @@ protected:
>  
>  	int init() override
>  	{
> -		CameraTest::init();
> +		int ret = CameraTest::init();
> +		if (ret)
> +			return ret;
>  
>  		defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!defconf_) {
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

On 10/06/2019 12:26, Niklas Söderlund wrote:
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Thanks, pushed with yours and Laurents tags.