From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BA1D1469719 for ; Sat, 31 Oct 2020 03:15:40 +0300 (MSK) References: <20201014133535.224573-1-gorcunov@gmail.com> <20201014133535.224573-3-gorcunov@gmail.com> <8f3da283-fc9a-31ce-cc96-e52963364a75@tarantool.org> <20201030101519.GE198833@grain> From: Vladislav Shpilevoy Message-ID: Date: Sat, 31 Oct 2020 01:15:38 +0100 MIME-Version: 1.0 In-Reply-To: <20201030101519.GE198833@grain> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v8 2/4] module_cache: move module handling into own subsystem List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml >>> + >>> +/** >>> + * Find a path to a module using Lua's package.cpath. >>> + * >>> + * @param ctx module find context >>> + * @param[out] ctx->path path to shared library. >>> + * @param[out] ctx->path_len size of @a ctx->path buffer. >> >> 2. The last two lines are not params. The only parameter is ctx. If >> you want to describe it, it should be done under the same @param. >> Otherwise the comment looks mismatching the signature. I gonna say >> that again. I don't force to use Doxygen, but if you decide to use it, >> it should be done correctly. > > Well, I didnt find any reference for the correct way to describe such > updates of internal structures. Doxygen syntax can be found on the official site, available on a first link from google when you type 'doxygen'. If it does not provide a special syntax for describing changes of individual fields of a single parameter, then you need to describe them in the comment main part, or the @param describing the parameter, like I proposed above. > I think I better drop this doxy-style > comment. Or drop it, yes. >>> + * >>> + * @return 0 on succes, -1 otherwise, diag is set. >>> + */ >>> +static int >>> +module_find(struct module_find_ctx *ctx) >>> diff --git a/src/box/module_cache.h b/src/box/module_cache.h >>> new file mode 100644 >>> index 000000000..3f275024a >>> --- /dev/null >>> +++ b/src/box/module_cache.h >>> @@ -0,0 +1,156 @@ >>> +/* >>> + * SPDX-License-Identifier: BSD-2-Clause >>> + * >>> + * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file. >>> + */ >>> + >>> +#pragma once >>> + >>> +#include >>> + >>> +#include "func_def.h" >> >> 3. Func_def heavily depends on schema. Also it is a part of >> all the _func handling, meaning this is a cyclic dependency now - >> module_cache depends on func and vice versa. > > Nope. Where you get it? func_def.h is not touched in this series > at all. What we have now I didn't say you touched it. I said it depends on schema, and is a part of stored functions code. Including func.h, func.c, call.h, call.c, and the others. > module_cache.h: func_def.h > func.h: module_cache.h func_def.h > func_def.h: trivia/util.h field_def.h opt_def.h > > I don't understand which cyclic dependency you mean. func.h/func.c/func_def.h/func_def.c are a part of the subsystem for calling stored functions. Inside of the schema. It also depends on module_cache. module_cache is not related to schema, but depends on func_def.h somewhy. This is the cycle. Module_cache and stored functions depend on each other. Talking of the schema details: func_def.h operates on user ID, on function ID, on Lua specifics like sandboxing, knows about multikey, and so on. It heavily depends on schema, and module_cache should not depend on schema. AFAIS, you included it simply because it defined box_function_ctx and box_function_f. But it should be vice-versa. I did this and all was built fine: ==================== diff --git a/src/box/func_def.h b/src/box/func_def.h index d99d89190..75cd6a0d3 100644 --- a/src/box/func_def.h +++ b/src/box/func_def.h @@ -168,20 +168,6 @@ func_def_dup(struct func_def *def); int func_def_check(struct func_def *def); -/** - * API of C stored function. - */ - -struct port; - -struct box_function_ctx { - struct port *port; -}; - -typedef struct box_function_ctx box_function_ctx_t; -typedef int (*box_function_f)(box_function_ctx_t *ctx, - const char *args, const char *args_end); - #ifdef __cplusplus } #endif diff --git a/src/box/module_cache.h b/src/box/module_cache.h index 3f275024a..1ec7f0042 100644 --- a/src/box/module_cache.h +++ b/src/box/module_cache.h @@ -8,12 +8,20 @@ #include -#include "func_def.h" - #if defined(__cplusplus) extern "C" { #endif /* defined(__cplusplus) */ +struct port; + +struct box_function_ctx { + struct port *port; +}; + +typedef struct box_function_ctx box_function_ctx_t; +typedef int (*box_function_f)(box_function_ctx_t *ctx, + const char *args, const char *args_end); + /** * Function name descriptor: a symbol and a package. */