[{"id":25299,"web_url":"https://patchwork.libcamera.org/comment/25299/","msgid":"","date":"2022-10-05T11:12:27","subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline_handler:\n\tReturn unique_ptr from createInstance","submitter":{"id":107,"url":"https://patchwork.libcamera.org/api/people/107/","name":"Xavier Roumegue","email":"xavier.roumegue@oss.nxp.com"},"content":"Hi Laurent,\n\nThanks for the patch.\n\nOn 10/3/22 23:21, Laurent Pinchart via libcamera-devel wrote:\n> Avoid naked pointer with memory allocation by returning a unique_ptr\n> from PipelineHandlerFactory::createInstance(), in order to increase\n> memory allocation safety.\n> \n> This allows iterating over factories in the CameraManager and unit tests\n> using const pointers.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/pipeline_handler.h | 8 +++++---\n> src/libcamera/camera_manager.cpp | 4 ++--\n> src/libcamera/pipeline_handler.cpp | 8 ++++----\n> test/ipa/ipa_interface_test.cpp | 4 ++--\n> 4 files changed, 13 insertions(+), 11 deletions(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index ebbdf2aa391f..ad74dc823290 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -113,7 +113,8 @@ public:\n> private:\n> \tstatic void registerType(PipelineHandlerFactory *factory);\n> \n> -\tvirtual PipelineHandler *createInstance(CameraManager *manager) const = 0;\n> +\tvirtual std::unique_ptr\n> +\tcreateInstance(CameraManager *manager) const = 0;\n> \n> \tstd::string name_;\n> };\n> @@ -125,9 +126,10 @@ public:\t\t\t\t\t\t\t\t\t\\\n> \thandler##Factory() : PipelineHandlerFactory(#handler) {}\t\\\n> \t\t\t\t\t\t\t\t\t\\\n> private:\t\t\t\t\t\t\t\t\\\n> -\tPipelineHandler *createInstance(CameraManager *manager) const\t\\\n> +\tstd::unique_ptr\t\t\t\t\\\n> +\tcreateInstance(CameraManager *manager) const\t\t\t\\\n> \t{\t\t\t\t\t\t\t\t\\\n> -\t\treturn new handler(manager);\t\t\t\t\\\n> +\t\treturn std::make_unique(manager);\t\t\\\n> \t}\t\t\t\t\t\t\t\t\\\n> };\t\t\t\t\t\t\t\t\t\\\n> static handler##Factory global_##handler##Factory;\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 7ff4bc6fd019..2c3f2d86c469 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -142,10 +142,10 @@ void CameraManager::Private::createPipelineHandlers()\n> \t * file and only fallback on all handlers if there is no\n> \t * configuration file.\n> \t */\n> -\tstd::vector &factories =\n> +\tconst std::vector &factories =\n> \t\tPipelineHandlerFactory::factories();\n> \n> -\tfor (PipelineHandlerFactory *factory : factories) {\n> +\tfor (const PipelineHandlerFactory *factory : factories) {\n> \t\tLOG(Camera, Debug)\n> \t\t\t<< \"Found registered pipeline handler '\"\n> \t\t\t<< factory->name() << \"'\";\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 4470e9041e8e..488dd8d32cdf 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -678,9 +678,9 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name)\n> */\n> std::shared_ptr PipelineHandlerFactory::create(CameraManager *manager) const\n> {\n> -\tPipelineHandler *handler = createInstance(manager);\n> +\tstd::unique_ptr handler = createInstance(manager);\n> \thandler->name_ = name_.c_str();\n> -\treturn std::shared_ptr(handler);\n> +\treturn std::shared_ptr(std::move(handler));\n> }\n> \n> /**\n> @@ -727,8 +727,8 @@ std::vector &PipelineHandlerFactory::factories()\n> * macro. It creates a pipeline handler instance associated with the camera\n> * \\a manager.\n> *\n> - * \\return a pointer to a newly constructed instance of the PipelineHandler\n> - * subclass corresponding to the factory\n> + * \\return A unique pointer to a newly constructed instance of the\n> + * PipelineHandler subclass corresponding to the factory\n> */\n> \n> /**\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index 3c0df843ea61..a629abc28da7 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -52,9 +52,9 @@ protected:\n> \t\tipaManager_ = make_unique();\n> \n> \t\t/* Create a pipeline handler for vimc. */\n> -\t\tstd::vector &factories =\n> +\t\tconst std::vector &factories =\n> \t\t\tPipelineHandlerFactory::factories();\n> -\t\tfor (PipelineHandlerFactory *factory : factories) {\n> +\t\tfor (const PipelineHandlerFactory *factory : factories) {\n> \t\t\tif (factory->name() == \"PipelineHandlerVimc\") {\n> \t\t\t\tpipe_ = factory->create(nullptr);\n> \t\t\t\tbreak;\n\nReviewed-by: Xavier Roumegue ","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 99D1EC0DA4\n\tfor ;\n\tWed, 5 Oct 2022 11:12:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 10DE8622EB;\n\tWed, 5 Oct 2022 13:12:34 +0200 (CEST)","from EUR02-DB5-obe.outbound.protection.outlook.com\n\t(mail-db5eur02on2060.outbound.protection.outlook.com [40.107.249.60])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A402601C7\n\tfor ;\n\tWed, 5 Oct 2022 13:12:33 +0200 (CEST)","from PAXPR04MB8703.eurprd04.prod.outlook.com\n\t(2603:10a6:102:21e::22)\n\tby PR3PR04MB7225.eurprd04.prod.outlook.com (2603:10a6:102:83::6) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5676.28;\n\tWed, 5 Oct 2022 11:12:30 +0000","from PAXPR04MB8703.eurprd04.prod.outlook.com\n\t([fe80::4f72:a35a:8c60:63f1]) by\n\tPAXPR04MB8703.eurprd04.prod.outlook.com\n\t([fe80::4f72:a35a:8c60:63f1%6]) with mapi id 15.20.5676.028;\n\tWed, 5 Oct 2022 11:12:29 +0000"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1664968354;\n\tbh=ixkRa69Viceb767Kksjf2bpfANPFUpvkMq68ugSh0qI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=f1+H6v0qPxOYJOzk6ifs/B25B4l0c7c8wbRjp4hnC6O8sR/k1B6CRgTKpuar5MRAD\n\trE9KBdJbjBmbM2dOAA/sqY+ozyDM95dHfLVwrl65G8OP2Nk21NhABlzGdicTttTOV1\n\tGEQlvWqwP46/aH9anKPD00B6v4gqfPXAvcQLjFd8eKfkXSeCE/+TzBFnJI4NKWvXG3\n\t5d2XlLiV4oxRC+axE/SVfgauZ2xHbecBTltF5BJG39Q6YsDDlx/EQ8N0dy9I573FOg\n\tp7CccT8CjsAc7Yx2qjdaXt9e3NdQ6wbpX/pKewWR3jxWLpCYf5ToQdf/aC1xl5ov29\n\tOoa5cKmiH8RIg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=NXP1.onmicrosoft.com;\n\ts=selector2-NXP1-onmicrosoft-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=Zg2Tj9z0W+KCU6RlnBHJbFisFtRbNi8jOKZ3LdAlZA4=;\n\tb=e8CRZowI9uf4OrEzfCIrCVORqZlPnRBsYnXQj56fSSUsncfU4MZGpYKePGoNgAdx1iQrhaFdkZ4yi/gnSV8tDqmCdtQeLkyp1WWSftoaIrHtJ7A8ue1Ftd7kXqj4/tNXOQev1pITw/kS3awdUZRuTxYJg4dcuCC7BggqdJ2aRSU="],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=NXP1.onmicrosoft.com\n\theader.i=@NXP1.onmicrosoft.com\n\theader.b=\"e8CRZowI\"; dkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=oss.nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;\n\tb=g3+QsCIwWokYQR4gukj+KHGWm5iR+JFS2bHwDxs2ta6ZpkTzOWbF/HsPZW3P2/HgVX02wHObb9SnIS6HoQ39LsfL0J64ft1em81QxfT21ONxq3wcgyBEGuBQ257ab0CpBP5+LUrgJ3gt+KR9Cn6iJv5tgDXlNWDxV0HgGIiA7KteDvJJEqhiQrVwFa1Oaf0K4BY7teM2icK/8bVUKFeJ5niOtYHCvpX0wXdD1SuD1M+z/obJ9L6wpJxw9OcU7BeByv/WEAUr1WseVSWPa9a4eEG6EGZ2FDIjLbYbwVayx4ZXJLSPleNt4bwRCbth7WIjNaYdMH5ApjZ6zzQ0ndFlXA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector9901;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=Zg2Tj9z0W+KCU6RlnBHJbFisFtRbNi8jOKZ3LdAlZA4=;\n\tb=Wm3BdXbzfy841Gs8nhtatjnkV6b2Kr1Hp2W83cN94r00q8T4zb9QvjGCVuUyyny4HuKxomasZXMRpJtDi9Y9CVVrl6A4deLWwB2luY7f57LduAPRSlDBNUoZVrgL4SRLiYXtdSKhp+YHByubBCg0JVO4h4GshxT73wKjh9OZp+Zhd8GJ+wdMxi/UjZj7Erpb8QC38DLL1EPThCjEWWfvo9TJkINYaSJvqf8fw1QpdEtxIsiS436VnHL4EbvRGie2KrUJOD7iPUKMfJhTnDLzsA5hgECuBITqRGVzBJsFIsMyTPKJGr0W8Ex9VnJ8LLkJ6AdKPyNhPmM8/lUBu1F8dw==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=oss.nxp.com;\n\tdmarc=pass action=none header.from=oss.nxp.com; \n\tdkim=pass header.d=oss.nxp.com; arc=none","Message-ID":"","Date":"Wed, 5 Oct 2022 13:12:27 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.3.1","Content-Language":"en-US, fr","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20221003212128.32429-1-laurent.pinchart@ideasonboard.com>\n\t<20221003212128.32429-8-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20221003212128.32429-8-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-ClientProxiedBy":"PR1P264CA0072.FRAP264.PROD.OUTLOOK.COM\n\t(2603:10a6:102:2cc::14) To PAXPR04MB8703.eurprd04.prod.outlook.com\n\t(2603:10a6:102:21e::22)","MIME-Version":"1.0","X-MS-Exchange-MessageSentRepresentingType":"1","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"PAXPR04MB8703:EE_|PR3PR04MB7225:EE_","X-MS-Office365-Filtering-Correlation-Id":"e0be0b3a-b921-4ccb-8f7b-08daa6c27d6e","X-MS-Exchange-SharedMailbox-RoutingAgent-Processed":"True","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;","X-Microsoft-Antispam-Message-Info":"0l0m0bhv2YEMwWribrJPUI9ZLcP+aQrpADywSFyQTmruSOs0aaY9EesXF8VJ0nfdugd1LuGVd6Y8x1pvD/7S8pdUKK6ZU6USiAIJr0nMMzBEFmUkiOW6ptE7WY2vLv8EzAv9niT6yIPQvQ8e0w0RV7wPaDUHT976koJzb3CLLNpL7U0lT6R48IdSlIqkU5kVKn3ym3UIx+01VY1brTcK126PWxmiGMkaWEQMvVV5GaWnGFXTSQNlnHFAxtwAwywbO7NtAqEtcNLMHcZMv042FLE8hsU1kps1EXEjgMO9rDlwANOXo5kMPt0Xa/PLn+/doFG7N4I0gFR1Ii+kSxTtBz2lvFDzjVqWxKbrZuPUDg70oOGidQN6TKodmlHad6L5lcBLpxuZ4G7M3Wj2A5nWomv/Bz9Og+vhlgHFm7M1OzL6HGWUrszIQ3rzrbuIiS1LQpELuGQGNCJkIS5qghCu9LR8M+TR9IVvUUDb7c2fGtad69aYILPphShb3NCtmXWYu9Dao23W7bsIZ3F83T1zZbvU5nc9rcamo/jLkQZ5Q0v08fL4RD436+qgVltM9jem//Tn3L8WLPoU1krxuEXn4P8nbCprwaPdBmk+yUT+e10jCoRnn/LlBbEonWn9NfAp7XsdxEhJCsMjsz/7GdGkLh8u9/8z0Q5S5J7w9mJWPL5cns2StOyd2QEMMp6mzqddBGkx6u1pplH5MPROlIeD9Nzoz15DjvioICOkruxAGSuxh2Xo2IV6mAByGcuPAhcu1M+PSjH+ohY7PCS0D5i/3UCBD4HfFJ70UswjVpaE4sA=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:PAXPR04MB8703.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230022)(4636009)(396003)(136003)(39860400002)(376002)(346002)(366004)(451199015)(66946007)(8676002)(66556008)(41300700001)(38100700002)(186003)(66476007)(2616005)(2906002)(8936002)(6512007)(83380400001)(31696002)(6506007)(86362001)(53546011)(5660300002)(52116002)(6486002)(316002)(478600001)(31686004)(43740500002)(45980500001);\n\tDIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?g7UE+M5WGEBJ0kJzO5vEa7N4G?=\n\t=?utf-8?q?jcSqrt9q9YO1P/6FnLEHAzPXuXJfWoQnZhRLvijpt21MYzTyL+dMVzXb?=\n\t=?utf-8?q?01ePhqEgMnDvvQ+bU2BwZ4KpQenk+qUGqlFMHHUKFuTu9y0xVM40eqfZ?=\n\t=?utf-8?q?FOU7tYiy+HWLEd3BgnwK/xXq3c1v4WyysaZ/+BBJOo+YiVmPS4OEumEb?=\n\t=?utf-8?q?5C/FJFMPQ/aud1g5AgVnDAJTOrtn2omzIbojQrApbr6Y7M53yhsKpr15?=\n\t=?utf-8?q?yXoSjyaJB803z2BkCLB4Jijkq1iTWbfHfz3agwn4S8g3RbvZHCe8XGKc?=\n\t=?utf-8?q?mYWsv8CrxTdsR3taRUwqvqoRnp8pn4oFe/6EVOSQ91Yr1Q3KIbe1w06K?=\n\t=?utf-8?q?dYQD6ANPMUCkvjj+GOD4AvDTIl22KO9GZLbSHWj6pb9KwqEhGrgZUiT9?=\n\t=?utf-8?q?W9oa8bizufFlJ/RKo+JezMB/UCfNqTIXOvcBE7jfOP+F3TrToRuULilT?=\n\t=?utf-8?q?14CZAOBo8h5E/TQews1IF9UQZD05rzKn8U8N0THpby17Jadrg3OU4ATl?=\n\t=?utf-8?q?4MWUeo0+VdDdb4o7lR0hLzX6grThxqEXQ5QI9eP7MfcKE8Qo3wGtJvFw?=\n\t=?utf-8?q?YzrysrV9cOmV/3JJK6i/AA+87G+Opns138sRP0u6zmcrblcTRTSlIXJq?=\n\t=?utf-8?q?yGVJL+uSBuWh/E0nWswaQXldEyTbFI8yPnMSdeXuR0mcYrxS4PhVvKbP?=\n\t=?utf-8?q?/VlgCCXV3FO5k5JlkXBM2FXqDJILHlFx0CVDvW+ujhLE78wS76cgZPBH?=\n\t=?utf-8?q?8ybGnpn2U4vzfAA50H4n6C2ffZj5/j3K06guPCHGRDvyoKkHRx7QeeZk?=\n\t=?utf-8?q?Vj1YfPjKJB4VHlm+Yts4wR6UwhExkbeyvjS0lv05oupad20LLtQ3IoYl?=\n\t=?utf-8?q?UprQrhJsc8t3x/Idu4S/wDvn3aSmiCl6HgbKl9ol93cJbB2lfa+v5Cvc?=\n\t=?utf-8?q?+p9hkbJ2eJ19rE+AVo8+Qz4p4v8pbrOEDshCTv0U3z+wPgP4O9vpeP6C?=\n\t=?utf-8?q?/nDAThgOTyF8s7Fr0O1ERhHzf4XxRZ9g+29W3YgZ8BHDpsOwdH/OgIiJ?=\n\t=?utf-8?q?4tO1nLpdO72qsrkQuV1rFDFpH1vbPYZ2QH6OQvxQPhE3IYylMRAhY3AI?=\n\t=?utf-8?q?mWLfhpMXJLjpwDwIINwY0gisUsyuRxWRJ+2rGRJM1ox9JOGhWUz6e80C?=\n\t=?utf-8?q?0tTj7X+XmKZ4f4PrLSL9hJoQGc5SXxbpehkF2jYkCXKeX+iUFxxiDq/2?=\n\t=?utf-8?q?d0o4LGTulh1KsBmg25DLN30Ic//QPeh70aL15B19mGT4v0BYnLrTm4lI?=\n\t=?utf-8?q?jpWId9Em52pchLiF3oGL2+a0obtYJ1NnDQpj4qcBIkK5S1vx1kHpOkL6?=\n\t=?utf-8?q?S7gcbawdyaqr5PTaz6aun+/hoDS3eXuEGEQR7UkDZbXl/yJVew4n6lD0?=\n\t=?utf-8?q?RBu3rdzNL9TZKAeUaww5J8L2bK3DHzkEcfQNpwg5wLeYRCIgbN3N7XQ/?=\n\t=?utf-8?q?qisSseE8Wlg/Dj6ODHhdR4+mdW/jktLfdVhetM/ucHQ68n2wyqXHKZqo?=\n\t=?utf-8?q?VFPOHlnsozkAK6rooXTYfzrszBpthLuWb6GEbvWzBz0z0VBQgwMmtc8f?=\n\t=?utf-8?q?O0BJAyL4ng9LG1E9t+dnpYNMi8Qrs0vx/8GwlkH7BrGznh9RP3Kib/gy?=\n\t=?utf-8?q?2SHycu5VHKWhxc3DzdSlzjD7lNO9YehRjDf7nGZLHIBrtzayCuuWIPw5?=\n\t=?utf-8?q?z+LlVRmZNT4nGfrpIKl+Dnmo5GZUSvAiVa3mA=3D=3D?=","X-OriginatorOrg":"oss.nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"e0be0b3a-b921-4ccb-8f7b-08daa6c27d6e","X-MS-Exchange-CrossTenant-AuthSource":"PAXPR04MB8703.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"05 Oct 2022 11:12:28.9622\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"gbwDTEmVEMglZXpi8mHGwTc3ohe/xjwP1AcCILATEjG7ILXthk0Xx25My6Wnf4/4ckwCMTTOOQcQ9XcIwo5jGw==","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"PR3PR04MB7225","Subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline_handler:\n\tReturn unique_ptr from createInstance","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","From":"\"Xavier Roumegue \\(OSS\\) via libcamera-devel\"\n\t","Reply-To":"\"Xavier Roumegue \\(OSS\\)\" ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25338,"web_url":"https://patchwork.libcamera.org/comment/25338/","msgid":"<20221007141707.iaruw57kega7rtcx@uno.localdomain>","date":"2022-10-07T14:17:07","subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline_handler:\n\tReturn unique_ptr from createInstance","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Oct 04, 2022 at 12:21:27AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Avoid naked pointer with memory allocation by returning a unique_ptr\n> from PipelineHandlerFactory::createInstance(), in order to increase\n> memory allocation safety.\n>\n> This allows iterating over factories in the CameraManager and unit tests\n> using const pointers.\n>\n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> include/libcamera/internal/pipeline_handler.h | 8 +++++---\n> src/libcamera/camera_manager.cpp | 4 ++--\n> src/libcamera/pipeline_handler.cpp | 8 ++++----\n> test/ipa/ipa_interface_test.cpp | 4 ++--\n> 4 files changed, 13 insertions(+), 11 deletions(-)\n>\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index ebbdf2aa391f..ad74dc823290 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -113,7 +113,8 @@ public:\n> private:\n> \tstatic void registerType(PipelineHandlerFactory *factory);\n>\n> -\tvirtual PipelineHandler *createInstance(CameraManager *manager) const = 0;\n> +\tvirtual std::unique_ptr\n> +\tcreateInstance(CameraManager *manager) const = 0;\n>\n> \tstd::string name_;\n> };\n> @@ -125,9 +126,10 @@ public:\t\t\t\t\t\t\t\t\t\\\n> \thandler##Factory() : PipelineHandlerFactory(#handler) {}\t\\\n> \t\t\t\t\t\t\t\t\t\\\n> private:\t\t\t\t\t\t\t\t\\\n> -\tPipelineHandler *createInstance(CameraManager *manager) const\t\\\n> +\tstd::unique_ptr\t\t\t\t\\\n> +\tcreateInstance(CameraManager *manager) const\t\t\t\\\n> \t{\t\t\t\t\t\t\t\t\\\n> -\t\treturn new handler(manager);\t\t\t\t\\\n> +\t\treturn std::make_unique(manager);\t\t\\\n> \t}\t\t\t\t\t\t\t\t\\\n> };\t\t\t\t\t\t\t\t\t\\\n> static handler##Factory global_##handler##Factory;\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 7ff4bc6fd019..2c3f2d86c469 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -142,10 +142,10 @@ void CameraManager::Private::createPipelineHandlers()\n> \t * file and only fallback on all handlers if there is no\n> \t * configuration file.\n> \t */\n> -\tstd::vector &factories =\n> +\tconst std::vector &factories =\n> \t\tPipelineHandlerFactory::factories();\n>\n> -\tfor (PipelineHandlerFactory *factory : factories) {\n> +\tfor (const PipelineHandlerFactory *factory : factories) {\n> \t\tLOG(Camera, Debug)\n> \t\t\t<< \"Found registered pipeline handler '\"\n> \t\t\t<< factory->name() << \"'\";\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 4470e9041e8e..488dd8d32cdf 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -678,9 +678,9 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name)\n> */\n> std::shared_ptr PipelineHandlerFactory::create(CameraManager *manager) const\n> {\n> -\tPipelineHandler *handler = createInstance(manager);\n> +\tstd::unique_ptr handler = createInstance(manager);\n> \thandler->name_ = name_.c_str();\n> -\treturn std::shared_ptr(handler);\n> +\treturn std::shared_ptr(std::move(handler));\n> }\n>\n> /**\n> @@ -727,8 +727,8 @@ std::vector &PipelineHandlerFactory::factories()\n> * macro. It creates a pipeline handler instance associated with the camera\n> * \\a manager.\n> *\n> - * \\return a pointer to a newly constructed instance of the PipelineHandler\n> - * subclass corresponding to the factory\n> + * \\return A unique pointer to a newly constructed instance of the\n> + * PipelineHandler subclass corresponding to the factory\n> */\n>\n> /**\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index 3c0df843ea61..a629abc28da7 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -52,9 +52,9 @@ protected:\n> \t\tipaManager_ = make_unique();\n>\n> \t\t/* Create a pipeline handler for vimc. */\n> -\t\tstd::vector &factories =\n> +\t\tconst std::vector &factories =\n> \t\t\tPipelineHandlerFactory::factories();\n> -\t\tfor (PipelineHandlerFactory *factory : factories) {\n> +\t\tfor (const PipelineHandlerFactory *factory : factories) {\n> \t\t\tif (factory->name() == \"PipelineHandlerVimc\") {\n> \t\t\t\tpipe_ = factory->create(nullptr);\n> \t\t\t\tbreak;\n> --\n> Regards,\n>\n> Laurent Pinchart\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 AC938BD16B\n\tfor ;\n\tFri, 7 Oct 2022 14:17:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE0ED62D19;\n\tFri, 7 Oct 2022 16:17:10 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9662360A88\n\tfor ;\n\tFri, 7 Oct 2022 16:17:09 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id E97552000C;\n\tFri, 7 Oct 2022 14:17:08 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665152231;\n\tbh=WGdUUIqkAepqQN3qFcf49H+LxYNSbTRbVAU1KU19iMY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1HBbOehRduvB+QHGvcrRhSR4bDaXWEbAjamV8Yi7A2FzAIxPy72HzjJoqvkcFYwoJ\n\tgHW+Sct9ZOWiE67ygUEweUX1wqKy0mAwcqb8WeDwO6td8clR1UJTiRXVAyVD2NlUZd\n\t/80BaU9E9rPZnFo7aqLjKNTfdcSr7y2IyUQ0t1YQ10EcJon1mx8XOeIayU8k1CPoK3\n\tut5CpTHDI+4ZzUxNGAHOOGiF+4rqwVmKdIuPGWrTQLrVywWsB9YR3pW28noRWd8G7P\n\t7XqjE6JsOciaECNL6k4eN6Gl5fvldhl0+RGC7y+LFAF0bJY6+FJgHVCs1EbxsbdYY2\n\tgeVVEssM4rVRQ==","Date":"Fri, 7 Oct 2022 16:17:07 +0200","To":"Laurent Pinchart ","Message-ID":"<20221007141707.iaruw57kega7rtcx@uno.localdomain>","References":"<20221003212128.32429-1-laurent.pinchart@ideasonboard.com>\n\t<20221003212128.32429-8-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221003212128.32429-8-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline_handler:\n\tReturn unique_ptr from createInstance","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]