[{"id":31094,"web_url":"https://patchwork.libcamera.org/comment/31094/","msgid":"<5c909c6e-b527-4827-9fe1-636216dca96d@ideasonboard.com>","date":"2024-09-05T11:00:59","subject":"Re: [PATCH v3 1/2] libcamera: utils: Add ScopeExitActions class","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 04/08/24 9:01 pm, Laurent Pinchart wrote:\n> The ScopeExitActions class is a simple object that performs\n> user-provided actions upon destruction. It is meant to simplify cleanup\n> tasks in error handling paths.\n>\n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Xavier Roumegue \n> Reviewed-by: Paul Elder \n\nReviewed-by: Umang Jain \n> ---\n> Changes since v2:\n>\n> - Reaplace the last references to \"defer\"\n>\n> Changes since v1:\n>\n> - Rename to ScopeExitActions\n> - Replace defer() with operator+=()\n> - Replace clear() with release()\n> ---\n> include/libcamera/base/utils.h | 13 ++++\n> src/libcamera/base/utils.cpp | 132 +++++++++++++++++++++++++++++++++\n> 2 files changed, 145 insertions(+)\n>\n> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h\n> index 734ff81e2860..b87f9b0ade7a 100644\n> --- a/include/libcamera/base/utils.h\n> +++ b/include/libcamera/base/utils.h\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -399,6 +400,18 @@ constexpr std::underlying_type_t to_underlying(Enum e) noexcept\n> \treturn static_cast>(e);\n> }\n> \n> +class ScopeExitActions\n> +{\n> +public:\n> +\t~ScopeExitActions();\n> +\n> +\tvoid operator+=(std::function &&action);\n> +\tvoid release();\n> +\n> +private:\n> +\tstd::vector> actions_;\n> +};\n> +\n> } /* namespace utils */\n> \n> #ifndef __DOXYGEN__\n> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp\n> index ccb31063b922..67e5a8964680 100644\n> --- a/src/libcamera/base/utils.cpp\n> +++ b/src/libcamera/base/utils.cpp\n> @@ -531,6 +531,138 @@ double strtod(const char *__restrict nptr, char **__restrict endptr)\n> * \\return The value of e converted to its underlying type\n> */\n> \n> +/**\n> + * \\class ScopeExitActions\n> + * \\brief An object that performs actions upon destruction\n> + *\n> + * The ScopeExitActions class is a simple object that performs user-provided\n> + * actions upon destruction. It is meant to simplify cleanup tasks in error\n> + * handling paths.\n> + *\n> + * When the code flow performs multiple sequential actions that each need a\n> + * corresponding cleanup action, error handling quickly become tedious:\n> + *\n> + * \\code{.cpp}\n> + * {\n> + * \tint ret = allocateMemory();\n> + * \tif (ret)\n> + * \t\treturn ret;\n> + *\n> + * \tret = startProducer();\n> + * \tif (ret) {\n> + * \t\tfreeMemory();\n> + * \t\treturn ret;\n> + * \t}\n> + *\n> + * \tret = startConsumer();\n> + * \tif (ret) {\n> + * \t\tstopProducer();\n> + * \t\tfreeMemory();\n> + * \t\treturn ret;\n> + * \t}\n> + *\n> + * \treturn 0;\n> + * }\n> + * \\endcode\n> + *\n> + * This is prone to programming mistakes, as cleanup actions can easily be\n> + * forgotten or ordered incorrectly. One strategy to simplify error handling is\n> + * to use goto statements:\n> + *\n> + * \\code{.cpp}\n> + * {\n> + * \tint ret = allocateMemory();\n> + * \tif (ret)\n> + * \t\treturn ret;\n> + *\n> + * \tret = startProducer();\n> + * \tif (ret)\n> + * \t\tgoto error_free;\n> + *\n> + * \tret = startConsumer();\n> + * \tif (ret)\n> + * \t\tgoto error_stop;\n> + *\n> + * \treturn 0;\n> + *\n> + * error_stop:\n> + * \tstopProducer();\n> + * error_free:\n> + * \tfreeMemory();\n> + * \treturn ret;\n> + * }\n> + * \\endcode\n> + *\n> + * While this may be considered better, this solution is still quite\n> + * error-prone. Beside the risk of picking the wrong error label, the error\n> + * handling logic is separated from the normal code flow, which increases the\n> + * risk of error when refactoring the code. Additionally, C++ doesn't allow\n> + * goto statements to jump over local variable declarations, which can make\n> + * usage of this pattern more difficult.\n> + *\n> + * The ScopeExitActions class solves these issues by allowing code that\n> + * requires cleanup actions to be grouped with its corresponding error handling\n> + * code:\n> + *\n> + * \\code{.cpp}\n> + * {\n> + * \tScopeExitActions actions;\n> + *\n> + * \tint ret = allocateMemory();\n> + * \tif (ret)\n> + * \t\treturn ret;\n> + *\n> + * \tactions += [&]() { freeMemory(); };\n> + *\n> + * \tret = startProducer();\n> + * \tif (ret)\n> + * \t\treturn ret;\n> + *\n> + * \tactions += [&]() { stopProducer(); };\n> + *\n> + * \tret = startConsumer();\n> + * \tif (ret)\n> + * \t\treturn ret;\n> + *\n> + * \tactions.release();\n> + * \treturn 0;\n> + * }\n> + * \\endcode\n> + *\n> + * Error handlers are executed when the ScopeExitActions instance is destroyed,\n> + * in the reverse order of their addition.\n> + */\n> +\n> +ScopeExitActions::~ScopeExitActions()\n> +{\n> +\tfor (const auto &action : utils::reverse(actions_))\n> +\t\taction();\n> +}\n> +\n> +/**\n> + * \\brief Add an exit action\n> + * \\param[in] action The action\n> + *\n> + * Add an exit action to the ScopeExitActions. Actions will be called upon\n> + * destruction in the reverse order of their addition.\n> + */\n> +void ScopeExitActions::operator+=(std::function &&action)\n> +{\n> +\tactions_.push_back(std::move(action));\n> +}\n> +\n> +/**\n> + * \\brief Remove all exit actions\n> + *\n> + * This function should be called in scope exit paths that don't need the\n> + * actions to be executed, such as success return paths from a function when\n> + * the ScopeExitActions is used for error cleanup.\n> + */\n> +void ScopeExitActions::release()\n> +{\n> +\tactions_.clear();\n> +}\n> +\n> } /* namespace utils */\n> \n> #ifndef __DOXYGEN__","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 15246C324C\n\tfor ;\n\tThu, 5 Sep 2024 11:01:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE784634E5;\n\tThu, 5 Sep 2024 13:01:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01F6D6345D\n\tfor ;\n\tThu, 5 Sep 2024 13:01:03 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 41D54AD0;\n\tThu, 5 Sep 2024 12:59:50 +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=\"ZgiQXAxW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725533990;\n\tbh=qIGPbxr8inszoGA8kruKNXGew8TacryvlbFnCfurPno=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ZgiQXAxWxBpJdYCeK1jmcn0EKt48TNE7w27c1q+mDIC2K0sk71oxQhW8NoN2Sbwbs\n\t1jT0WR99y/otqT4yt3zzkkOFUFuSzqyjFa+i4j3vO6rj+UYcx+Kw+qf4Wp5ppj3FAr\n\tHskDec5d+Ahvd6Tjj+9941ELjkKCMkkx57NeInoY=","Message-ID":"<5c909c6e-b527-4827-9fe1-636216dca96d@ideasonboard.com>","Date":"Thu, 5 Sep 2024 16:30:59 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 1/2] libcamera: utils: Add ScopeExitActions class","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240804153106.22167-1-laurent.pinchart@ideasonboard.com>\n\t<20240804153106.22167-2-laurent.pinchart@ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20240804153106.22167-2-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]