[v2] libcamera: request: addBuffer(): Do not destroy fence on failure
diff mbox series

Message ID 20250910151857.1502329-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v2] libcamera: request: addBuffer(): Do not destroy fence on failure
Related show

Commit Message

Barnabás Pőcze Sept. 10, 2025, 3:18 p.m. UTC
Take the unique pointer to the `Fence` object by rvalue reference
so that it is not destroyed if the function returns an error code
and does not take ownership of the unique pointer.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
changes in v2:
  * modify function documentation

v1: https://patchwork.libcamera.org/patch/22907/
---
 include/libcamera/request.h | 2 +-
 src/libcamera/request.cpp   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

--
2.51.0

Comments

Laurent Pinchart Sept. 10, 2025, 4:45 p.m. UTC | #1
On Wed, Sep 10, 2025 at 05:18:57PM +0200, Barnabás Pőcze wrote:
> Take the unique pointer to the `Fence` object by rvalue reference
> so that it is not destroyed if the function returns an error code
> and does not take ownership of the unique pointer.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> ---
> changes in v2:
>   * modify function documentation
> 
> v1: https://patchwork.libcamera.org/patch/22907/
> ---
>  include/libcamera/request.h | 2 +-
>  src/libcamera/request.cpp   | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index e214a9d13..0c5939f7b 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -53,7 +53,7 @@ public:
>  	ControlList &metadata() { return *metadata_; }
>  	const BufferMap &buffers() const { return bufferMap_; }
>  	int addBuffer(const Stream *stream, FrameBuffer *buffer,
> -		      std::unique_ptr<Fence> fence = nullptr);
> +		      std::unique_ptr<Fence> &&fence = {});
>  	FrameBuffer *findBuffer(const Stream *stream) const;
> 
>  	uint32_t sequence() const;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 7f1e11e8f..26bba8f28 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -452,7 +452,9 @@ void Request::reuse(ReuseFlag flags)
>   *
>   * When a valid Fence is provided to this function, \a fence is moved to \a
>   * buffer and this Request will only be queued to the device once the
> - * fences of all its buffers have been correctly signalled.
> + * fences of all its buffers have been correctly signalled. Ownership of the
> + * fence will only be taken in case of success, otherwise the fence will
> + * be left unmodified.
>   *
>   * If the \a fence associated with \a buffer isn't signalled, the request will
>   * fail after a timeout. The buffer will still contain the fence, which
> @@ -468,7 +470,7 @@ void Request::reuse(ReuseFlag flags)
>   * \retval -EINVAL The buffer does not reference a valid Stream
>   */
>  int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,
> -		       std::unique_ptr<Fence> fence)
> +		       std::unique_ptr<Fence> &&fence)
>  {
>  	if (!stream) {
>  		LOG(Request, Error) << "Invalid stream reference";

Patch
diff mbox series

diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index e214a9d13..0c5939f7b 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -53,7 +53,7 @@  public:
 	ControlList &metadata() { return *metadata_; }
 	const BufferMap &buffers() const { return bufferMap_; }
 	int addBuffer(const Stream *stream, FrameBuffer *buffer,
-		      std::unique_ptr<Fence> fence = nullptr);
+		      std::unique_ptr<Fence> &&fence = {});
 	FrameBuffer *findBuffer(const Stream *stream) const;

 	uint32_t sequence() const;
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 7f1e11e8f..26bba8f28 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -452,7 +452,9 @@  void Request::reuse(ReuseFlag flags)
  *
  * When a valid Fence is provided to this function, \a fence is moved to \a
  * buffer and this Request will only be queued to the device once the
- * fences of all its buffers have been correctly signalled.
+ * fences of all its buffers have been correctly signalled. Ownership of the
+ * fence will only be taken in case of success, otherwise the fence will
+ * be left unmodified.
  *
  * If the \a fence associated with \a buffer isn't signalled, the request will
  * fail after a timeout. The buffer will still contain the fence, which
@@ -468,7 +470,7 @@  void Request::reuse(ReuseFlag flags)
  * \retval -EINVAL The buffer does not reference a valid Stream
  */
 int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,
-		       std::unique_ptr<Fence> fence)
+		       std::unique_ptr<Fence> &&fence)
 {
 	if (!stream) {
 		LOG(Request, Error) << "Invalid stream reference";