[Tarantool-patches] [PATCH] net.box: allow to store user-defined fields in future object
Vladimir Davydov
vdavydov at tarantool.org
Fri Aug 13 13:01:46 MSK 2021
On Thu, Aug 12, 2021 at 09:02:46PM +0300, Vladislav Shpilevoy wrote:
> > +
> > +static int
> > +luaT_netbox_request_newindex(struct lua_State *L)
> > +{
> > + struct netbox_request *request = luaT_check_netbox_request(L, 1);
> > + struct mh_strnptr_t *h = request->index;
> > + size_t field_name_len;
> > + const char *field_name = lua_tolstring(L, 2, &field_name_len);
> > + if (field_name == NULL)
> > + return luaL_error(L, "invalid index");
> > + int field_value_ref = luaL_ref(L, LUA_REGISTRYINDEX);
> > + if (field_value_ref == LUA_REFNIL) {
> > + /* The field is set to nil. Delete it from the index. */
>
> 1. But it is not deleted. It is just ignored, which is fine, but
> the comment is wrong then.
>
> > + if (h == NULL)
> > + return 0;
> > + mh_int_t k = mh_strnptr_find_inp(h, field_name,
> > + field_name_len);
> > + int ref = (intptr_t)mh_strnptr_node(h, k)->val;
> > + luaL_unref(L, LUA_REGISTRYINDEX, ref);
> > + mh_strnptr_del(h, k, NULL);
> > + return 0;
> > + }
> > + if (h == NULL) {
> > + /* Lazily create the index on the first invocation. */
> > + h = request->index = mh_strnptr_new();
> > + if (h == NULL) {
> > + luaL_unref(L, LUA_REGISTRYINDEX, field_value_ref);
> > + return luaL_error(L, "out of memory");
> > + }
> > + }
> > + /* Insert a reference to the new value into the index. */
> > + struct mh_strnptr_node_t node = {
> > + .str = field_name,
> > + .len = field_name_len,
> > + .hash = mh_strn_hash(field_name, field_name_len),
>
> 2. lua_hashstring() might be faster. You could reuse already
> calculated hash value from Lua. Although for that 'field_name'
> must be of exactly string type (lua_tolstring() works not only for
> strings). So if you would make a more precise type check, you
> could avoid hash calculation completely. Or use lua_hash() for
> non-string values' strings. (But I might be wrong about how
> lua_hashstring() works for non-string values, you would need to
> double-check).
>
> > + .val = (void *)(intptr_t)field_value_ref,
>
> 3. Can you simply cast to void * right away? Why do you need the
> intermediate intptr_t cast?
I reworked the patch to use a Lua table instead of mhash, as per
the suggestion by imun@, so these comments are not relevant anymore.
Please see v3:
https://lists.tarantool.org/pipermail/tarantool-patches/2021-August/025458.html
More information about the Tarantool-patches
mailing list