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 v8 1/4] box/func: factor out c function entry structure Date: Sat, 31 Oct 2020 18:27:30 +0300 [thread overview] Message-ID: <20201031152730.GG198833@grain> (raw) In-Reply-To: <1c716ed7-8782-2d0a-2f60-d8ec7c4e95d9@tarantool.org> On Sat, Oct 31, 2020 at 01:13:18AM +0100, Vladislav Shpilevoy wrote: > >>> + module_delete(module); > >>> + return -1; > >>> + } > >>> + } > >>> + > >>> + mod_sym->addr = module_sym(module, name.sym); > >>> + if (mod_sym->addr == NULL) > >>> + return -1; > >> > >> 2. If the module was loaded first time here, it is not unloaded in case of > >> an error in this place. > > > > Just like it was before the patch. The patch simply factor outs the old > > code. It doesn't improve it (because, lets be honest this is a min problem > > for module management -- we've to check for module symbol not at moment > > of calling it but rather at the moment when we load a function). That > > said the issue with module management is known and I think we need to > > rework modules code more deeply, but not in this series. In the > > series it remains exactly as it was. > > There is no even a ticket for that. And if you add it, it will go to wishlist, > I can assure you. The rule is that if you see a bug, the first thing you do is > ensure there is a ticket on it. This issue is definitely a bug. So you can't rely > on some refactoring of something sometime in the future. If there is a bug, it > must not be lost, and must be fixed. Bugs have higher priority than refactoring > almost always, and there is more chances it will be fixed when filed separately > from refactoring. > > https://github.com/tarantool/tarantool/issues/5475 I assigned this bug for myself. I see your point, and I rather tend to agree in general. I'll wix. > >> > >> 3. Can't parse the comment. What is the 'C callback'? > >> And why is this function stored? After you extracted it > >> from struct func_c, it is not related to _func space, and > >> is not stored. > > > > It is stored in memory. > > Everything is stored in memory in Tarantool except Vinyl. I > see the comment is just copy-pasted from the previous place > in struct func_c, where it meant stored as stored in the > schema. In spaces. > > > The C callback is the function we > > call, so we keep a pointer to a module. > > You don't need a module pointer to call a function. It can be > called by (), without a module. It is saved here to load it and > unload. At unload it may delete the module if the function object > was the last user of the module. Seems we don't understad each other. This is a symbol we retrieve from the module and we literally "call" it then. The symbol has a name and an address, and address used as callable object. I'm not sure how else I could rephrase it.
next prev parent reply other threads:[~2020-10-31 15:27 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-14 13:35 [Tarantool-patches] [PATCH v8 0/4] box/cbox: implement cfunc Lua module Cyrill Gorcunov 2020-10-14 13:35 ` [Tarantool-patches] [PATCH v8 1/4] box/func: factor out c function entry structure Cyrill Gorcunov 2020-10-29 22:15 ` Vladislav Shpilevoy 2020-10-30 9:51 ` Cyrill Gorcunov 2020-10-31 0:13 ` Vladislav Shpilevoy 2020-10-31 15:27 ` Cyrill Gorcunov [this message] 2020-10-14 13:35 ` [Tarantool-patches] [PATCH v8 2/4] module_cache: move module handling into own subsystem Cyrill Gorcunov 2020-10-29 22:15 ` Vladislav Shpilevoy 2020-10-30 10:15 ` Cyrill Gorcunov 2020-10-31 0:15 ` Vladislav Shpilevoy 2020-10-31 15:29 ` Cyrill Gorcunov 2020-10-14 13:35 ` [Tarantool-patches] [PATCH v8 3/4] box/cbox: implement cbox Lua module Cyrill Gorcunov 2020-10-29 22:15 ` Vladislav Shpilevoy 2020-10-30 12:51 ` Cyrill Gorcunov 2020-10-31 0:21 ` Vladislav Shpilevoy 2020-10-31 21:59 ` Cyrill Gorcunov 2020-11-01 8:26 ` Cyrill Gorcunov 2020-11-02 22:25 ` Vladislav Shpilevoy 2020-11-03 7:26 ` Cyrill Gorcunov 2020-11-12 22:54 ` Vladislav Shpilevoy 2020-11-13 18:30 ` Cyrill Gorcunov 2020-10-14 13:35 ` [Tarantool-patches] [PATCH v8 4/4] 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=20201031152730.GG198833@grain \ --to=gorcunov@gmail.com \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v8 1/4] box/func: factor out c function entry structure' \ /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