Re: [Tarantool-patches] [PATCH v20 3/7] box/func: fix modules functions restore

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

> 
> 02.04.2021 15:34, Cyrill Gorcunov пишет:
> > In commit 96938fafb an ability to hot reload of modules
> > has been introduced. When module is been reloaded his
> 
> typo: either 'is being reloaded' or 'has been reloaded'.

Thanks! Surely it should be "is being".

> > -	 * Some old-dso func can't be load from new module, restore old
> > -	 * functions.
> > +	 * Some old functions are not found int the new module,
> > +	 * thus restore all migrated functions back to original.
> 
> typo: in the new module.

Thanks!

> > @@ -419,9 +420,7 @@ module_reload(const char *package, const char *package_end)
> >   		}
> >   		func->module = old_module;
> >   		rlist_move(&old_module->funcs, &func->item);
> > -	} while (func != rlist_first_entry(&old_module->funcs,
> > -					   struct func_c, item));
> > -	assert(rlist_empty(&new_module->funcs));
> 
> Let's keep this assert.

The cycle is removing entries from the list and for this purpose
the cycle is using a "safe" form. I would prefer to drop this
useless assert() because a few lines above we are doing entries
removing. Frankly I don't understand why it has been introduced
in first place, maybe because this stange and buggy form of
"func != rlist_first_entry", dunno.

So if there is no really strong reason why should we keep this
assert I would love to drop it.

Look, here is what we have

	list_for_each_entry_safe(item, head, ...)
		list_move(item, new_head)

a few obvious lines. And list is clear once the @item reaches @head.

But if you still think it is better to keep and I didn't convince
you, then sure, I'll bring it back.