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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

>>> +			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

>>> +/**
>>> + * Callable symbol bound to a module.
>>> + */
>>> +struct module_sym {
>>> +	/**
>>> +	 * Anchor for module membership.
>>> +	 */
>>> +	struct rlist item;
>>> +	/**
>>> +	 * For C functions, address of the function.
>>> +	 */
>>> +	box_function_f addr;
>>> +	/**
>>> +	 * Each stored function keeps a handle to the
>>> +	 * dynamic library for the C callback.
>>> +	 */
>>
>> 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.