From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Mon, 10 Jun 2019 12:19:40 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v2 2/9] box: move box_module_reload routine to func.c
Message-ID: <20190610091940.bebqg3a2usk6vsnn@esperanza>
References: <cover.1559822429.git.kshcherbatov@tarantool.org>
 <4c036f20e844e4060e37583e85c28fb91d02156c.1559822429.git.kshcherbatov@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4c036f20e844e4060e37583e85c28fb91d02156c.1559822429.git.kshcherbatov@tarantool.org>
To: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Thu, Jun 06, 2019 at 03:03:58PM +0300, Kirill Shcherbatov wrote:
> Previously, the box_module_reload function was located in the
> box/call module that is wrong. It is moved to a more suitable
> place in box/func.
> ---
>  src/box/call.c     | 21 ---------------------
>  src/box/call.h     | 10 ----------
>  src/box/func.c     | 25 ++++++++++++++++++++++++-
>  src/box/func.h     | 12 ++++--------
>  src/box/lua/call.c |  1 +
>  5 files changed, 29 insertions(+), 40 deletions(-)
> 
> diff --git a/src/box/call.c b/src/box/call.c
> index 56da53fb3..982c95cbf 100644
> --- a/src/box/call.c
> +++ b/src/box/call.c
> @@ -132,27 +132,6 @@ box_c_call(struct func *func, struct call_request *request, struct port *port)
>  	return 0;
>  }
>  
> -int
> -box_module_reload(const char *name)
> -{
> -	struct credentials *credentials = effective_user();
> -	if ((credentials->universal_access & (PRIV_X | PRIV_U)) !=
> -	    (PRIV_X | PRIV_U)) {
> -		struct user *user = user_find(credentials->uid);
> -		if (user != NULL)
> -			diag_set(AccessDeniedError, priv_name(PRIV_U),
> -				 schema_object_name(SC_UNIVERSE), "",
> -				 user->def->name);
> -		return -1;
> -	}
> -	struct module *module = NULL;
> -	if (module_reload(name, name + strlen(name), &module) == 0 &&
> -	    module != NULL)
> -		return 0;
> -	diag_set(ClientError, ER_NO_SUCH_MODULE, name);
> -	return -1;
> -}
> -
>  int
>  box_process_call(struct call_request *request, struct port *port)
>  {
> diff --git a/src/box/call.h b/src/box/call.h
> index 1b54551be..4b3b8614c 100644
> --- a/src/box/call.h
> +++ b/src/box/call.h
> @@ -42,16 +42,6 @@ struct box_function_ctx {
>  	struct port *port;
>  };
>  
> -/**
> - * Reload loadable module by name.
> - *
> - * @param name of the module to reload.
> - * @retval -1 on error.
> - * @retval 0 on success.
> - */
> -int
> -box_module_reload(const char *name);
> -
>  int
>  box_process_call(struct call_request *request, struct port *port);
>  
> diff --git a/src/box/func.c b/src/box/func.c
> index a817851fd..098bc02b6 100644
> --- a/src/box/func.c
> +++ b/src/box/func.c
> @@ -31,6 +31,7 @@
>  #include "func.h"
>  #include "trivia/config.h"
>  #include "assoc.h"
> +#include "session.h"
>  #include "lua/utils.h"
>  #include "error.h"
>  #include "diag.h"
> @@ -298,7 +299,16 @@ module_sym(struct module *module, const char *name)
>  	return f;
>  }
>  
> -int
> +/**
> + * Reload dynamically loadable module.
> + *
> + * @param package name begin pointer.
> + * @param package_end package_end name end pointer.
> + * @param[out] module a pointer to store module object on success.
> + * @retval -1 on error.
> + * @retval 0 on success.
> + */
> +static int
>  module_reload(const char *package, const char *package_end, struct module **module)
>  {
>  	struct module *old_module = module_cache_find(package, package_end);
> @@ -355,6 +365,19 @@ restore:
>  	return -1;
>  }
>  
> +int
> +box_module_reload(const char *name)
> +{
> +	if (access_check_universe(PRIV_X | PRIV_U) != 0)
> +		return -1;
> +	struct module *module = NULL;
> +	if (module_reload(name, name + strlen(name), &module) == 0 &&
> +	    module != NULL)
> +		return 0;
> +	diag_set(ClientError, ER_NO_SUCH_MODULE, name);
> +	return -1;
> +}
> +

TBO I don't see any point in this change at this time - the function
could as well stay in src/box/call.c, along with all permission checks.
Please try to avoid stray changes so as not to pollute the git history.