From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Mon, 10 Jun 2019 13:32:47 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v2 4/9] box: rework func object as a function frontend
Message-ID: <20190610103247.d7aisnd6ibilkfvv@esperanza>
References: <cover.1559822429.git.kshcherbatov@tarantool.org>
 <8902e130a9cd1ac295391fc9cd315ad4cfa2bafa.1559822429.git.kshcherbatov@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8902e130a9cd1ac295391fc9cd315ad4cfa2bafa.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:04:00PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/func.c b/src/box/func.c
> +/** Private virtual method table for func object. */
> +struct func_vtab {
> +	/** Load function runtime environment. */
> +	int (*load)(struct func *func);

'load' could be a part of 'call' or constructor.

> +	/** Unload function runtime environment. */
> +	void (*unload)(struct func *func);

Let's call this 'destroy' and call it from func_delete.

> +	/** Call function with given arguments. */
> +	int (*call)(struct func *func, struct port *port, const char *args,
> +		    const char *args_end);
> +	/**
> +	 * Capture a given old_func's runtime and reuse in a given
> +	 * new function if possible.
> +	 */
> +	void (*reuse_runtime)(struct func *new_func, struct func *old_func);
> +};

If we drop func_update, we won't need this method.

> +int
> +box_func_execute_by_name(const char *name, uint32_t name_len, struct port *port,
> +			 const char *args, const char *args_end)
> +{

I think this should live in src/box/call.c:

	box_process_call:

	- look up function by name
	- if found call it
	- otherwise check X permission on Universe and call function by
	  name using the corresponding helper defined in box/lua/call.c.

> diff --git a/src/box/func.h b/src/box/func.h
> index 82ac046b7..931718bba 100644
> --- a/src/box/func.h
> +++ b/src/box/func.h
> @@ -42,6 +42,9 @@
>  extern "C" {
>  #endif /* defined(__cplusplus) */
>  
> +struct port;
> +struct func_vtab;
> +
>  /**
>   * Dynamic shared module.
>   */
> @@ -61,6 +64,8 @@ struct module {
>   */
>  struct func {
>  	struct func_def *def;
> +	/** Virtual method table. */
> +	struct func_vtab *vtab;
>  	/**
>  	 * Anchor for module membership.
>  	 */
> @@ -68,7 +73,7 @@ struct func {
>  	/**
>  	 * For C functions, the body of the function.
>  	 */
> -	box_function_f func;
> +	box_function_f c_func;

I think it would be better to inherit the base struct so as not to mix
C/Lua stuff in the same struct:

	struct func {
		struct func_vtab *def;
		...
	};

	struct func_c {
		struct func base;
		box_function_f func;
		struct rlist in_module;
		...
	};