[libcamera-devel] [PATCH v3 22/22] libcamera: pipeline: rkisp1: Use cleanup error paths for start()

[Jacopo Mondi]

Hi Niklas,

On Fri, Sep 25, 2020 at 03:42:07AM +0200, Niklas Söderlund wrote:
> Use a lambda function to make the error paths a bit cleaner in start(),
> no functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 98 ++++++++++++------------
>  1 file changed, 51 insertions(+), 47 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c6c2e3aa3dc6d0dc..34196af3057b14bb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -827,58 +827,62 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  	RkISP1CameraData *data = cameraData(camera);
>  	int ret;
>
> -	/* Allocate buffers for internal pipeline usage. */
> -	ret = allocateBuffers(camera);
> -	if (ret)
> -		return ret;
> -
> -	ret = data->ipa_->start();
> -	if (ret) {
> -		freeBuffers(camera);
> -		LOG(RkISP1, Error)
> -			<< "Failed to start IPA " << camera->id();
> -		return ret;
> -	}
> -
> -	data->frame_ = 0;
> -
> -	ret = param_->streamOn();
> -	if (ret) {
> -		data->ipa_->stop();
> -		freeBuffers(camera);
> -		LOG(RkISP1, Error)
> -			<< "Failed to start parameters " << camera->id();
> -		return ret;
> -	}
> -
> -	ret = stat_->streamOn();
> -	if (ret) {
> -		param_->streamOff();
> -		data->ipa_->stop();
> -		freeBuffers(camera);
> -		LOG(RkISP1, Error)
> -			<< "Failed to start statistics " << camera->id();
> -		return ret;
> -	}
> -
> -	ret = mainPath_.start();
> -	if (ret) {
> -		param_->streamOff();
> -		stat_->streamOff();
> -		data->ipa_->stop();
> -		freeBuffers(camera);
> -		return ret;
> -	}
> -
> -	ret = selfPath_.start();
> -	if (ret) {
> +	auto startDevices = [this](Camera *camera, RkISP1CameraData *data) {

Why a lamda function for a function that is only called for a single place as a
regulr function ?

Also, shouldn't re-working the existing code to use labels be better ?

> +		int ret;
> +
> +		/* Allocate buffers for internal pipeline usage. */
> +		ret = allocateBuffers(camera);
> +		if (ret)
> +			return ret;
> +
> +		ret = data->ipa_->start();
> +		if (ret) {
> +			LOG(RkISP1, Error)
> +				<< "Failed to start IPA " << camera->id();
> +			goto err_allocate;
> +		}
> +
> +		data->frame_ = 0;
> +
> +		ret = param_->streamOn();
> +		if (ret) {
> +			LOG(RkISP1, Error)
> +				<< "Failed to start parameters " << camera->id();
> +			goto err_ipa;
> +		}
> +
> +		ret = stat_->streamOn();
> +		if (ret) {
> +			LOG(RkISP1, Error)
> +				<< "Failed to start statistics " << camera->id();
> +			goto err_param;
> +		}
> +
> +		ret = mainPath_.start();
> +		if (ret)
> +			goto err_stat;
> +
> +		ret = selfPath_.start();
> +		if (ret)
> +			goto err_main;
> +
> +		return 0;
> +	err_main:
>  		mainPath_.stop();
> -		param_->streamOff();
> +	err_stat:
>  		stat_->streamOff();
> +	err_param:
> +		param_->streamOff();
> +	err_ipa:
>  		data->ipa_->stop();
> +	err_allocate:
>  		freeBuffers(camera);
>  		return ret;
> -	}
> +	};
> +
> +	ret = startDevices(camera, data);
> +	if (ret)
> +		return ret;
>
>  	activeCamera_ = camera;
>
> --
> 2.28.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel