[Tarantool-patches] [PATCH v10 1/4] box/func: factor out c function entry structure

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Nov 13 01:53:40 MSK 2020

Thanks for the patch!

On 05.11.2020 16:18, Cyrill Gorcunov wrote:
> Currently func_c structure is a wrapper over struct func
> which in turn handles function definition, credentials and etc,
> mostly to handle internal "_func" space.
> Such tight bound doesn't allow to reuse func_c in any
> other context. But we're going to support C function
> execution in read-only instances and for this we better
> reuse already existing structures as much as possible
> instead of breeding new ones.
> Thus lets extract module symbols into module_sym structure,
> this allows us to reuse module path cache in other code.
> While extracting the structure rename "func" member to
> "addr" since this exactly what the member represents:
> an address of symbol in memory.
> Same time to be completely independent of "_func" specific lets
> carry symbolic name inplace (ie member "name" is kind of redundant
> when module_sym is used for "_func" handling where we can retrieve
> the name via function definition but such definition won't be
> available for non-persistent C functions which we will support
> in next patches).
> The new structure allows us to load/unload general symbols via
> module_sym_load() and module_sym_unload() helpers.
> While been factoring out Vlad reported that we've an issue:
> if a module has been loaded for a first time but doesn't have
> symbol requested it will remain in memory keeping a reference
> to a shared library for nothing.

Please, never squash refatoring and a bugfix into a single commit.
That makes the commit not atomic, and creates problems with
backporting such commit, which is clearly needed here.

Move the bugfix to a separate commit which can be cherry-picked to
the older versions.

Point 3, "Don't do a change if it is not related to the patch".
The issues 5475 and 4642 are not related.

It is not a cargo cult rule. I just really don't see the actual fix in
all this refactoring, because too much code is moved, and only some
tiny part of it is changed.

More information about the Tarantool-patches mailing list