[v3,1/2] libcamera: utils: Add ScopeExitActions class

20759 diff mbox series

The ScopeExitActions class is a simple object that performs
user-provided actions upon destruction. It is meant to simplify cleanup
tasks in error handling paths.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes since v2:

- Reaplace the last references to "defer"

Changes since v1:

- Rename to ScopeExitActions
- Replace defer() with operator+=()
- Replace clear() with release()
---
 include/libcamera/base/utils.h |  13 ++++
 src/libcamera/base/utils.cpp   | 132 +++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+)

Hi Laurent,

On 04/08/24 9:01 pm, Laurent Pinchart wrote:
> The ScopeExitActions class is a simple object that performs
> user-provided actions upon destruction. It is meant to simplify cleanup
> tasks in error handling paths.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> Changes since v2:
>
> - Reaplace the last references to "defer"
>
> Changes since v1:
>
> - Rename to ScopeExitActions
> - Replace defer() with operator+=()
> - Replace clear() with release()
> ---
>   include/libcamera/base/utils.h |  13 ++++
>   src/libcamera/base/utils.cpp   | 132 +++++++++++++++++++++++++++++++++
>   2 files changed, 145 insertions(+)
>
> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> index 734ff81e2860..b87f9b0ade7a 100644
> --- a/include/libcamera/base/utils.h
> +++ b/include/libcamera/base/utils.h
> @@ -9,6 +9,7 @@
>   
>   #include <algorithm>
>   #include <chrono>
> +#include <functional>
>   #include <iterator>
>   #include <memory>
>   #include <ostream>
> @@ -399,6 +400,18 @@ constexpr std::underlying_type_t<Enum> to_underlying(Enum e) noexcept
>   	return static_cast<std::underlying_type_t<Enum>>(e);
>   }
>   
> +class ScopeExitActions
> +{
> +public:
> +	~ScopeExitActions();
> +
> +	void operator+=(std::function<void()> &&action);
> +	void release();
> +
> +private:
> +	std::vector<std::function<void()>> actions_;
> +};
> +
>   } /* namespace utils */
>   
>   #ifndef __DOXYGEN__
> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> index ccb31063b922..67e5a8964680 100644
> --- a/src/libcamera/base/utils.cpp
> +++ b/src/libcamera/base/utils.cpp
> @@ -531,6 +531,138 @@ double strtod(const char *__restrict nptr, char **__restrict endptr)
>    * \return The value of e converted to its underlying type
>    */
>   
> +/**
> + * \class ScopeExitActions
> + * \brief An object that performs actions upon destruction
> + *
> + * The ScopeExitActions class is a simple object that performs user-provided
> + * actions upon destruction. It is meant to simplify cleanup tasks in error
> + * handling paths.
> + *
> + * When the code flow performs multiple sequential actions that each need a
> + * corresponding cleanup action, error handling quickly become tedious:
> + *
> + * \code{.cpp}
> + * {
> + * 	int ret = allocateMemory();
> + * 	if (ret)
> + * 		return ret;
> + *
> + * 	ret = startProducer();
> + * 	if (ret) {
> + * 		freeMemory();
> + * 		return ret;
> + * 	}
> + *
> + * 	ret = startConsumer();
> + * 	if (ret) {
> + * 		stopProducer();
> + * 		freeMemory();
> + * 		return ret;
> + * 	}
> + *
> + * 	return 0;
> + * }
> + * \endcode
> + *
> + * This is prone to programming mistakes, as cleanup actions can easily be
> + * forgotten or ordered incorrectly. One strategy to simplify error handling is
> + * to use goto statements:
> + *
> + * \code{.cpp}
> + * {
> + * 	int ret = allocateMemory();
> + * 	if (ret)
> + * 		return ret;
> + *
> + * 	ret = startProducer();
> + * 	if (ret)
> + * 		goto error_free;
> + *
> + * 	ret = startConsumer();
> + * 	if (ret)
> + * 		goto error_stop;
> + *
> + * 	return 0;
> + *
> + * error_stop:
> + * 	stopProducer();
> + * error_free:
> + * 	freeMemory();
> + * 	return ret;
> + * }
> + * \endcode
> + *
> + * While this may be considered better, this solution is still quite
> + * error-prone. Beside the risk of picking the wrong error label, the error
> + * handling logic is separated from the normal code flow, which increases the
> + * risk of error when refactoring the code. Additionally, C++ doesn't allow
> + * goto statements to jump over local variable declarations, which can make
> + * usage of this pattern more difficult.
> + *
> + * The ScopeExitActions class solves these issues by allowing code that
> + * requires cleanup actions to be grouped with its corresponding error handling
> + * code:
> + *
> + * \code{.cpp}
> + * {
> + * 	ScopeExitActions actions;
> + *
> + * 	int ret = allocateMemory();
> + * 	if (ret)
> + * 		return ret;
> + *
> + * 	actions += [&]() { freeMemory(); };
> + *
> + * 	ret = startProducer();
> + * 	if (ret)
> + * 		return ret;
> + *
> + * 	actions += [&]() { stopProducer(); };
> + *
> + * 	ret = startConsumer();
> + * 	if (ret)
> + * 		return ret;
> + *
> + * 	actions.release();
> + * 	return 0;
> + * }
> + * \endcode
> + *
> + * Error handlers are executed when the ScopeExitActions instance is destroyed,
> + * in the reverse order of their addition.
> + */
> +
> +ScopeExitActions::~ScopeExitActions()
> +{
> +	for (const auto &action : utils::reverse(actions_))
> +		action();
> +}
> +
> +/**
> + * \brief Add an exit action
> + * \param[in] action The action
> + *
> + * Add an exit action to the ScopeExitActions. Actions will be called upon
> + * destruction in the reverse order of their addition.
> + */
> +void ScopeExitActions::operator+=(std::function<void()> &&action)
> +{
> +	actions_.push_back(std::move(action));
> +}
> +
> +/**
> + * \brief Remove all exit actions
> + *
> + * This function should be called in scope exit paths that don't need the
> + * actions to be executed, such as success return paths from a function when
> + * the ScopeExitActions is used for error cleanup.
> + */
> +void ScopeExitActions::release()
> +{
> +	actions_.clear();
> +}
> +
>   } /* namespace utils */
>   
>   #ifndef __DOXYGEN__