[v2,3/8] pipeline: rkisp1: Replace error handling gotos with utils::exit_scope
diff mbox series

Message ID 20250815113400.20623-4-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Use span in FrameBuffer & assorted cleanups
Related show

Commit Message

Laurent Pinchart Aug. 15, 2025, 11:33 a.m. UTC 
Use utils::exit_scope in PipelineHandlerRkISP1::allocateBuffers() to
avoid gotos for error handling.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Barnabás Pőcze Aug. 15, 2025, 12:15 p.m. UTC | #1 
2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:
> Use utils::exit_scope in PipelineHandlerRkISP1::allocateBuffers() to
> avoid gotos for error handling.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 81370f4cdcba..55d7d4442caf 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1002,21 +1002,27 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>   	unsigned int ipaBufferId = 1;
>   	int ret;
>   
> +	auto errorCleanup = utils::scope_exit{ [&]() {
> +		paramBuffers_.clear();
> +		statBuffers_.clear();
> +		mainPathBuffers_.clear();
> +	} };
> +
>   	if (!isRaw_) {
>   		ret = param_->allocateBuffers(kRkISP1MinBufferCount, &paramBuffers_);
>   		if (ret < 0)
> -			goto error;
> +			return ret;
>   
>   		ret = stat_->allocateBuffers(kRkISP1MinBufferCount, &statBuffers_);
>   		if (ret < 0)
> -			goto error;
> +			return ret;
>   	}
>   
>   	/* If the dewarper is being used, allocate internal buffers for ISP. */
>   	if (useDewarper_) {
>   		ret = mainPath_.exportBuffers(kRkISP1MinBufferCount, &mainPathBuffers_);
>   		if (ret < 0)
> -			goto error;
> +			return ret;
>   
>   		for (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)
>   			availableMainPathBuffers_.push(buffer.get());
> @@ -1038,14 +1044,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>   
>   	data->ipa_->mapBuffers(data->ipaBuffers_);
>   
> +	errorCleanup.release();
>   	return 0;
> -
> -error:
> -	paramBuffers_.clear();
> -	statBuffers_.clear();
> -	mainPathBuffers_.clear();
> -
> -	return ret;
>   }
>   
>   int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
Jacopo Mondi Aug. 19, 2025, 3:23 p.m. UTC | #2 
Hi Laurent

On Fri, Aug 15, 2025 at 02:33:55PM +0300, Laurent Pinchart wrote:
> Use utils::exit_scope in PipelineHandlerRkISP1::allocateBuffers() to
> avoid gotos for error handling.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 81370f4cdcba..55d7d4442caf 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1002,21 +1002,27 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  	unsigned int ipaBufferId = 1;
>  	int ret;
>
> +	auto errorCleanup = utils::scope_exit{ [&]() {
> +		paramBuffers_.clear();
> +		statBuffers_.clear();
> +		mainPathBuffers_.clear();
> +	} };
> +
>  	if (!isRaw_) {
>  		ret = param_->allocateBuffers(kRkISP1MinBufferCount, &paramBuffers_);
>  		if (ret < 0)
> -			goto error;
> +			return ret;
>
>  		ret = stat_->allocateBuffers(kRkISP1MinBufferCount, &statBuffers_);
>  		if (ret < 0)
> -			goto error;
> +			return ret;
>  	}
>
>  	/* If the dewarper is being used, allocate internal buffers for ISP. */
>  	if (useDewarper_) {
>  		ret = mainPath_.exportBuffers(kRkISP1MinBufferCount, &mainPathBuffers_);
>  		if (ret < 0)
> -			goto error;
> +			return ret;
>
>  		for (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)
>  			availableMainPathBuffers_.push(buffer.get());
> @@ -1038,14 +1044,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>
>  	data->ipa_->mapBuffers(data->ipaBuffers_);
>
> +	errorCleanup.release();
>  	return 0;
> -
> -error:
> -	paramBuffers_.clear();
> -	statBuffers_.clear();
> -	mainPathBuffers_.clear();
> -
> -	return ret;
>  }
>
>  int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 81370f4cdcba..55d7d4442caf 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1002,21 +1002,27 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 	unsigned int ipaBufferId = 1;
 	int ret;
 
+	auto errorCleanup = utils::scope_exit{ [&]() {
+		paramBuffers_.clear();
+		statBuffers_.clear();
+		mainPathBuffers_.clear();
+	} };
+
 	if (!isRaw_) {
 		ret = param_->allocateBuffers(kRkISP1MinBufferCount, &paramBuffers_);
 		if (ret < 0)
-			goto error;
+			return ret;
 
 		ret = stat_->allocateBuffers(kRkISP1MinBufferCount, &statBuffers_);
 		if (ret < 0)
-			goto error;
+			return ret;
 	}
 
 	/* If the dewarper is being used, allocate internal buffers for ISP. */
 	if (useDewarper_) {
 		ret = mainPath_.exportBuffers(kRkISP1MinBufferCount, &mainPathBuffers_);
 		if (ret < 0)
-			goto error;
+			return ret;
 
 		for (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)
 			availableMainPathBuffers_.push(buffer.get());
@@ -1038,14 +1044,8 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 
 	data->ipa_->mapBuffers(data->ipaBuffers_);
 
+	errorCleanup.release();
 	return 0;
-
-error:
-	paramBuffers_.clear();
-	statBuffers_.clear();
-	mainPathBuffers_.clear();
-
-	return ret;
 }
 
 int PipelineHandlerRkISP1::freeBuffers(Camera *camera)