[libcamera-devel,v2] libcamera: pipeline: simple: Only connect created converters
diff mbox series

Message ID 20201023101154.673143-1-kieran.bingham@ideasonboard.com
State Accepted
Commit 7adf3a069839c1b2c440b07ca2911ffc2d39e816
Headers show
Series
  • [libcamera-devel,v2] libcamera: pipeline: simple: Only connect created converters
Related show

Commit Message

Kieran Bingham Oct. 23, 2020, 10:11 a.m. UTC
If converter_->open() fails, the code deletes the converter_ but then
happily goes on, and at the very next lines will use converter_ to
connect the bufferReady signal.

Ensure the converter is only connected when successfully opened by
extending the conditional and connecting in an else scope instead.

Reported-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Oct. 23, 2020, 11:44 p.m. UTC | #1
Hi Kieran,

Thank you for the patch.

On Fri, Oct 23, 2020 at 11:11:54AM +0100, Kieran Bingham wrote:
> If converter_->open() fails, the code deletes the converter_ but then
> happily goes on, and at the very next lines will use converter_ to
> connect the bufferReady signal.
> 
> Ensure the converter is only connected when successfully opened by
> extending the conditional and connecting in an else scope instead.
> 
> Reported-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 0d48e1b6acca..3d2039f3f269 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -774,9 +774,9 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>  				<< "Failed to open converter, disabling format conversion";
>  			delete converter_;
>  			converter_ = nullptr;
> +		} else {
> +			converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);
>  		}
> -
> -		converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);
>  	}
>  
>  	/*

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 0d48e1b6acca..3d2039f3f269 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -774,9 +774,9 @@  bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 				<< "Failed to open converter, disabling format conversion";
 			delete converter_;
 			converter_ = nullptr;
+		} else {
+			converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);
 		}
-
-		converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);
 	}
 
 	/*