[libcamera-devel,3/5] Documentation: application-developer: Recommend unique_ptr for CameraManager
diff mbox series

Message ID 20210825102937.3740405-4-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • application-developer: Clean up
Related show

Commit Message

Kieran Bingham Aug. 25, 2021, 10:29 a.m. UTC
The CameraManager object should be deleted when it is no longer used to
prevent it from leaking.

When the application closes, the memory will be released, but it would
show up in reports from memory validation tools such as valgrind if not
handled correctly.

Recommend best-practices in the guide and ensure it is automatically
cleaned up when the CameraManager goes out of scope.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 Documentation/guides/application-developer.rst | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Aug. 25, 2021, 7:32 p.m. UTC | #1
Hi Kieran,

Thank you for the patch.

On Wed, Aug 25, 2021 at 11:29:35AM +0100, Kieran Bingham wrote:
> The CameraManager object should be deleted when it is no longer used to
> prevent it from leaking.
> 
> When the application closes, the memory will be released, but it would
> show up in reports from memory validation tools such as valgrind if not
> handled correctly.
> 
> Recommend best-practices in the guide and ensure it is automatically
> cleaned up when the CameraManager goes out of scope.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  Documentation/guides/application-developer.rst | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst
> index 442d8e6a512e..e2ed79d5173e 100644
> --- a/Documentation/guides/application-developer.rst
> +++ b/Documentation/guides/application-developer.rst
> @@ -61,11 +61,15 @@ variable for the camera to support the event call back later:
>     std::shared_ptr<Camera> camera;
>  
>  Create a Camera Manager instance at the beginning of the main function, and then
> -start it. An application should only create a single Camera Manager instance.
> +start it. An application must only create a single Camera Manager instance.
> +
> +The CameraManager can be stored in a unique_ptr to automate deleting the
> +instance when it is no longer used, but care must be taken to ensure all cameras

Line wrap.

> +are released explicitly.

I'd add "before this happens" at the end.

>  
>  .. code:: cpp
>  
> -   CameraManager *cm = new CameraManager();
> +   std::unique_ptr<CameraManager> cm = std::make_unique<CameraManager>();
>     cm->start();
>  
>  During the application initialization, the Camera Manager is started to
> @@ -560,6 +564,9 @@ uses, so needs to do the following:
>  
>     return 0;
>  
> +In this instance the CameraManager will automatically be deleted by the
> +unique_ptr implementation when it goes out of scope.

This feels a bit like teaching C++ :-) I probably wouldn't have included
this here, but I don't mind. Either way,

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

> +
>  Build and run instructions
>  --------------------------
>

Patch
diff mbox series

diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst
index 442d8e6a512e..e2ed79d5173e 100644
--- a/Documentation/guides/application-developer.rst
+++ b/Documentation/guides/application-developer.rst
@@ -61,11 +61,15 @@  variable for the camera to support the event call back later:
    std::shared_ptr<Camera> camera;
 
 Create a Camera Manager instance at the beginning of the main function, and then
-start it. An application should only create a single Camera Manager instance.
+start it. An application must only create a single Camera Manager instance.
+
+The CameraManager can be stored in a unique_ptr to automate deleting the
+instance when it is no longer used, but care must be taken to ensure all cameras
+are released explicitly.
 
 .. code:: cpp
 
-   CameraManager *cm = new CameraManager();
+   std::unique_ptr<CameraManager> cm = std::make_unique<CameraManager>();
    cm->start();
 
 During the application initialization, the Camera Manager is started to
@@ -560,6 +564,9 @@  uses, so needs to do the following:
 
    return 0;
 
+In this instance the CameraManager will automatically be deleted by the
+unique_ptr implementation when it goes out of scope.
+
 Build and run instructions
 --------------------------