From d8adc5286373f366c74c56a53cbb4258e9b315d3 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Mon, 13 Jan 2025 15:40:58 +0300 Subject: [PATCH] MDEV-22441 SCOPE_VALUE macro for temporary values - Needless engaged_ removed; - SCOPE_VALUE, SCOPE_SET, SCOPE_CLEAR macros for neater declaration; - IF_CLASS / IF_NOT_CLASS SFINAE checkers to pass arg by value or reference; - inline keyword; - couple of refactorings of temporary free_list. --- include/scope.h | 83 ++++++++++++++++++++++++++++++++++++---------- sql/sql_acl.cc | 4 +-- sql/sql_prepare.cc | 21 +++++------- sql/sql_show.cc | 2 +- 4 files changed, 76 insertions(+), 34 deletions(-) diff --git a/include/scope.h b/include/scope.h index 836c4f77b35..932ab410d78 100644 --- a/include/scope.h +++ b/include/scope.h @@ -32,6 +32,11 @@ public: { } + template + scope_exit(F &&f, bool engaged) : function_(std::forward(f)), engaged_(engaged) + { + } + scope_exit(scope_exit &&rhs) : function_(std::move(rhs.function_)), engaged_(rhs.engaged_) { @@ -43,6 +48,7 @@ public: scope_exit &operator=(const scope_exit &)= delete; void release() { engaged_= false; } + void engage() { DBUG_ASSERT(!engaged_); engaged_= true; } ~scope_exit() { @@ -58,38 +64,51 @@ private: } // end namespace detail template -detail::scope_exit::type> -make_scope_exit(Callable &&f) +inline +::detail::scope_exit::type> +make_scope_exit(Callable &&f, bool engaged= true) { - return detail::scope_exit::type>( - std::forward(f)); + return ::detail::scope_exit::type>( + std::forward(f), engaged); } #define CONCAT_IMPL(x, y) x##y - #define CONCAT(x, y) CONCAT_IMPL(x, y) - #define ANONYMOUS_VARIABLE CONCAT(_anonymous_variable, __LINE__) #define SCOPE_EXIT auto ANONYMOUS_VARIABLE= make_scope_exit +#define IF_CLASS(C) typename std::enable_if::value>::type +#define IF_NOT_CLASS(C) typename std::enable_if::value>::type + namespace detail { -template class Scope_value +template +class Scope_value { public: + // Use SFINAE for passing structs by reference and plain types by value. + // This ctor is defined only if T is a class or struct: + template Scope_value(T &variable, const T &scope_value) - : variable_(variable), saved_value_(variable) + : variable_(&variable), saved_value_(variable) + { + variable= scope_value; + } + + // This ctor is defined only if T is NOT a class or struct: + template + Scope_value(T &variable, const T scope_value) + : variable_(&variable), saved_value_(variable) { variable= scope_value; } Scope_value(Scope_value &&rhs) - : variable_(rhs.variable_), saved_value_(rhs.saved_value_), - engaged_(rhs.engaged_) + : variable_(rhs.variable_), saved_value_(rhs.saved_value_) { - rhs.engaged_= false; + rhs.variable_= NULL; } Scope_value(const Scope_value &)= delete; @@ -98,22 +117,50 @@ public: ~Scope_value() { - if (engaged_) - variable_= saved_value_; + if (variable_) + *variable_= saved_value_; } private: - T &variable_; + T *variable_; T saved_value_; - bool engaged_= true; }; } // namespace detail // Use like this: // auto _= make_scope_value(var, tmp_value); -template -detail::Scope_value make_scope_value(T &variable, const T &scope_value) + +template +inline +::detail::Scope_value make_scope_value(T &variable, const T &scope_value) { - return detail::Scope_value(variable, scope_value); + return ::detail::Scope_value(variable, scope_value); } + +template +inline +::detail::Scope_value make_scope_value(T &variable, T scope_value) +{ + return ::detail::Scope_value(variable, scope_value); +} + +/* + Note: perfect forwarding version can not pass const: + + template + inline + detail::Scope_value make_scope_value(T &variable, U &&scope_value) + { + return detail::Scope_value(variable, std::forward(scope_value)); + } + + as `const U &&` fails with error `expects an rvalue for 2nd argument`. That + happens because const U && is treated as rvalue only (this is the exact syntax + for declaring rvalues). +*/ + + +#define SCOPE_VALUE auto ANONYMOUS_VARIABLE= make_scope_value +#define SCOPE_SET(VAR, MASK) auto ANONYMOUS_VARIABLE= make_scope_value(VAR, VAR | MASK) +#define SCOPE_CLEAR(VAR, MASK) auto ANONYMOUS_VARIABLE= make_scope_value(VAR, VAR & ~MASK) diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 98e79babcc3..c7f2a8d3d46 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2555,9 +2555,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables) char tmp_name[SAFE_NAME_LEN+1]; DBUG_ENTER("acl_load"); - auto _= make_scope_value(thd->variables.sql_mode, - thd->variables.sql_mode & - ~MODE_PAD_CHAR_TO_FULL_LENGTH); + SCOPE_CLEAR(thd->variables.sql_mode, MODE_PAD_CHAR_TO_FULL_LENGTH); grant_version++; /* Privileges updated */ diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 9d53d7cf543..c102e2f734e 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2993,8 +2993,8 @@ void mysql_sql_stmt_execute_immediate(THD *thd) DBUG_VOID_RETURN; // out of memory // See comments on thd->free_list in mysql_sql_stmt_execute() - Item *free_list_backup= thd->free_list; - thd->free_list= NULL; + SCOPE_VALUE(thd->free_list, (Item *) NULL); + SCOPE_EXIT([thd]() mutable { thd->free_items(); }); /* Make sure we call Prepared_statement::execute_immediate() with an empty THD::change_list. It can be non empty as the above @@ -3017,8 +3017,6 @@ void mysql_sql_stmt_execute_immediate(THD *thd) Item_change_list_savepoint change_list_savepoint(thd); (void) stmt->execute_immediate(query.str, (uint) query.length); change_list_savepoint.rollback(thd); - thd->free_items(); - thd->free_list= free_list_backup; /* stmt->execute_immediately() sets thd->query_string with the executed @@ -3578,8 +3576,13 @@ void mysql_sql_stmt_execute(THD *thd) so they don't get freed in case of re-prepare. See MDEV-10702 Crash in SET STATEMENT FOR EXECUTE */ - Item *free_list_backup= thd->free_list; - thd->free_list= NULL; // Hide the external (e.g. "SET STATEMENT") Items + /* + Hide and restore at scope exit the "external" (e.g. "SET STATEMENT") Item list. + It will be freed normaly in THD::cleanup_after_query(). + */ + SCOPE_VALUE(thd->free_list, (Item *) NULL); + // Free items created by execute_loop() at scope exit + SCOPE_EXIT([thd]() mutable { thd->free_items(); }); /* Make sure we call Prepared_statement::execute_loop() with an empty THD::change_list. It can be non-empty because the above @@ -3603,12 +3606,6 @@ void mysql_sql_stmt_execute(THD *thd) (void) stmt->execute_loop(&expanded_query, FALSE, NULL, NULL); change_list_savepoint.rollback(thd); - thd->free_items(); // Free items created by execute_loop() - /* - Now restore the "external" (e.g. "SET STATEMENT") Item list. - It will be freed normaly in THD::cleanup_after_query(). - */ - thd->free_list= free_list_backup; stmt->lex->restore_set_statement_var(); DBUG_VOID_RETURN; diff --git a/sql/sql_show.cc b/sql/sql_show.cc index ee3d85e6ba2..dff2a9c73da 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -6438,7 +6438,7 @@ bool store_schema_params(THD *thd, TABLE *table, TABLE *proc_table, { Field *field; LEX_CSTRING tmp_string; - auto _= make_scope_value(thd->variables.sql_mode, sql_mode); + SCOPE_VALUE(thd->variables.sql_mode, sql_mode); if (sph->type() == SP_TYPE_FUNCTION) {