From: Cyrill Gorcunov <gorcunov@gmail.com> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tml <tarantool-patches@dev.tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v5 5/6] box/cbox: implement cbox Lua module Date: Fri, 9 Oct 2020 09:57:55 +0300 [thread overview] Message-ID: <20201009065755.GJ2069@grain> (raw) In-Reply-To: <3cc78ca9-5bf1-0128-1a09-313256be9253@tarantool.org> On Fri, Oct 09, 2020 at 12:35:09AM +0200, Vladislav Shpilevoy wrote: > > + > > +#include "box/error.h" > > +#include "box/func.h" > > 1. Why do you still depend on box/func.h? Func.h is > about stored functions with credentials and stuff. In the > previous review I proposed to move needed parts to a new > system: module_sym of whatever it is called. So it would > be module_sym.h and module_sym.c. And it would not depend > on schema at all. Currently func.h brings schema dependency > into cbox. Well, I remember your proposal. The reason is indeed to include module_sym structure which sits in func.h. I thought we could make this separation later together with fix of lazy address loading. But on the other hands let me try to implement it today. > Maybe a good name for the new system would be module_cache.h > and module_cache.c. Like we have with coll_id_cache.h/.c. > (The structs can remain module_sym.) Sounds reasonable. > > + > > + int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); > > + lua_xmove(L, args_L, lua_gettop(L) - 1); > > 2. Why didn't you use lua_remove() to get rid of the unnecessary > parts of the stack? Which one you think is redundant? The object itself? If so then it sits as a bottom element of the stack and removing it would simply make code slower. Vlad this "prepare environment for calling" part of code is very sensitive to the arguments, I tried a various combinations in attempts to get rid of lua thread and none of them work. So I left it as is and put a FIXME, because it requires more serious rework. Or I misundertood you, and you meant something different? > > + > > + lua_pushstring(L, "__call"); > > + lua_pushcfunction(L, lcbox_func_call); > > + lua_settable(L, -3); > > + > > + lua_setmetatable(L, -2); > > 3. What happens when the function object is deleted by Lua GC? > It seems the user can't get the function again without dropping > it. And that is a good question. I must confess I missed Lua GC aspect here. Thanks a huge! I'll rework!
next prev parent reply other threads:[~2020-10-09 6:57 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-08 21:36 [Tarantool-patches] [PATCH v5 0/6] box/cbox: implement cfunc " Cyrill Gorcunov 2020-10-08 21:36 ` [Tarantool-patches] [PATCH v5 1/6] box/func: factor out c function entry structure Cyrill Gorcunov 2020-10-08 21:36 ` [Tarantool-patches] [PATCH v5 2/6] box/func: provide module_sym_call Cyrill Gorcunov 2020-10-08 21:36 ` [Tarantool-patches] [PATCH v5 3/6] box/func: more detailed error in module reloading Cyrill Gorcunov 2020-10-08 21:36 ` [Tarantool-patches] [PATCH v5 4/6] box/func: export func_split_name helper Cyrill Gorcunov 2020-10-08 21:36 ` [Tarantool-patches] [PATCH v5 5/6] box/cbox: implement cbox Lua module Cyrill Gorcunov 2020-10-08 22:35 ` Vladislav Shpilevoy 2020-10-09 6:57 ` Cyrill Gorcunov [this message] 2020-10-09 21:31 ` Vladislav Shpilevoy 2020-10-11 21:34 ` Cyrill Gorcunov 2020-10-09 21:46 ` Vladislav Shpilevoy 2020-10-11 21:58 ` Cyrill Gorcunov 2020-10-08 21:36 ` [Tarantool-patches] [PATCH v5 6/6] test: box/cfunc -- add simple module test Cyrill Gorcunov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201009065755.GJ2069@grain \ --to=gorcunov@gmail.com \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v5 5/6] box/cbox: implement cbox Lua module' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox