maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Eugene!

On Jun 03, Eugene Kosov wrote:
> revision-id: e2017fc3a5f (mariadb-10.2.31-219-ge2017fc3a5f)

This should go into 10.5, not into old-old GA 10.2.
Besides, 10.2 cannot use C++11 features, 10.5 can.

> parent(s): 1b3adaab25c
> author: Eugene Kosov <claprix@xxxxxxxxx>
> committer: Eugene Kosov <eugene.kosov@xxxxxxxxxxx>
> timestamp: 2020-05-27 12:45:41 +0300
> message:
> 
> MDEV-22441 implement a generic way to change a value of a variable in scope
> 
> Scope_value<T> class changes a variable value to some temporary value while
> it exits and restores an old value in it's destructor.
> 
> make_scope_value() is a function targeted for C++11 which allows to omit
> a template parameter which is required by Scope_value<T>

I think I'd name it backup_and_change_scoped() or something like that.
But it's just a suggestion, feel free to keep make_scope_value() name if
you like it better.

> Sql_mode_save removed and the only usage is replaced with Scope_value<T>
> 
> Sql_mode_save removal should not be merged in newer branches!
> 
> ---
>  include/scope.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  sql/sql_acl.cc  |  7 ++++---
>  sql/sql_class.h | 16 ----------------
>  3 files changed, 50 insertions(+), 19 deletions(-)
> 
> diff --git a/include/scope.h b/include/scope.h
> new file mode 100644
> index 00000000000..81061ceed37
> --- /dev/null
> +++ b/include/scope.h
> @@ -0,0 +1,46 @@
> +/*
> +   Copyright (c) 2020, MariaDB Corporation.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; version 2 of the License.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1335  USA */
> +
> +#pragma once

use include guards, please, not a pragma

> +
> +namespace maria

and, as you remember, we didn't quite decide on a global server-wide
namespace, so it's not the MDEV that should introduce it.

> +{
> +
> +template <typename T> class Scope_value
> +{
> +public:
> +  Scope_value(T &variable, const T &scope_value)
> +      : variable_(variable), saved_value_(variable)
> +  {
> +    variable= scope_value;
> +  }
> +
> +  ~Scope_value() { variable_= saved_value_; }
> +
> +private:
> +  T &variable_;
> +  T saved_value_;
> +};
> +
> +// Starting from C++11 this allows to omit template argument like this:
> +// auto _ = make_scope_value(var, tmp_value);
> +template <typename T>
> +Scope_value<T> make_scope_value(T &variable, const T &scope_value)
> +{
> +  return Scope_value<T>(variable, scope_value);
> +}

as you've seen from Sql_mode_save usage, sometimes the value is assigned
later, not when it's saved. Please, add another helper that doesn't
require to provide a value. Like

 template <typename T>
 Scope_value<T> backup_scoped(T &variable)
 {
   return Scope_value<T>(variable, variable);
 }

> +
> +} // namespace maria

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx