[{"id":19064,"web_url":"https://patchwork.libcamera.org/comment/19064/","msgid":"","date":"2021-08-25T19:32:05","subject":"Re: [libcamera-devel] [PATCH 3/5] Documentation:\n\tapplication-developer: Recommend unique_ptr for CameraManager","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Wed, Aug 25, 2021 at 11:29:35AM +0100, Kieran Bingham wrote:\n> The CameraManager object should be deleted when it is no longer used to\n> prevent it from leaking.\n> \n> When the application closes, the memory will be released, but it would\n> show up in reports from memory validation tools such as valgrind if not\n> handled correctly.\n> \n> Recommend best-practices in the guide and ensure it is automatically\n> cleaned up when the CameraManager goes out of scope.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> Documentation/guides/application-developer.rst | 11 +++++++++--\n> 1 file changed, 9 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst\n> index 442d8e6a512e..e2ed79d5173e 100644\n> --- a/Documentation/guides/application-developer.rst\n> +++ b/Documentation/guides/application-developer.rst\n> @@ -61,11 +61,15 @@ variable for the camera to support the event call back later:\n> std::shared_ptr camera;\n> \n> Create a Camera Manager instance at the beginning of the main function, and then\n> -start it. An application should only create a single Camera Manager instance.\n> +start it. An application must only create a single Camera Manager instance.\n> +\n> +The CameraManager can be stored in a unique_ptr to automate deleting the\n> +instance when it is no longer used, but care must be taken to ensure all cameras\n\nLine wrap.\n\n> +are released explicitly.\n\nI'd add \"before this happens\" at the end.\n\n> \n> .. code:: cpp\n> \n> - CameraManager *cm = new CameraManager();\n> + std::unique_ptr cm = std::make_unique();\n> cm->start();\n> \n> During the application initialization, the Camera Manager is started to\n> @@ -560,6 +564,9 @@ uses, so needs to do the following:\n> \n> return 0;\n> \n> +In this instance the CameraManager will automatically be deleted by the\n> +unique_ptr implementation when it goes out of scope.\n\nThis feels a bit like teaching C++ :-) I probably wouldn't have included\nthis here, but I don't mind. Either way,\n\nReviewed-by: Laurent Pinchart \n\n> +\n> Build and run instructions\n> --------------------------\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BD6DCBD87D\n\tfor ;\n\tWed, 25 Aug 2021 19:32:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2CB5A688A5;\n\tWed, 25 Aug 2021 21:32:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9FBB060288\n\tfor ;\n\tWed, 25 Aug 2021 21:32:17 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2568424F;\n\tWed, 25 Aug 2021 21:32:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"reNPVuAO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629919937;\n\tbh=sMM3gDcAeqPk03UglAFsHQkZ8TVUrMjBb0AiE4dav9A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=reNPVuAOjpGXVUFwfBgEnkvtFnj7k7UHQ9TU3nhEUYVsG1ukFjn9+5/mN9rFPhwGY\n\tvbH9mJbpDMlF+xzwG0GarG6JyHMhWsHXhXliN9X54DNnhdl+Jo46+XUX26wKorD81G\n\t75ThHDRywBMf2xtzLUDSw4DIuCRc4nTZUMAgOKxA=","Date":"Wed, 25 Aug 2021 22:32:05 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210825102937.3740405-1-kieran.bingham@ideasonboard.com>\n\t<20210825102937.3740405-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210825102937.3740405-4-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/5] Documentation:\n\tapplication-developer: Recommend unique_ptr for CameraManager","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]