From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 2/3] Add module to ease using Lua iterators from C Date: Mon, 31 Dec 2018 09:43:56 +0300 [thread overview] Message-ID: <20181231064355.fsfb7om3p7yqieqv@tkn_work_nb> (raw) In-Reply-To: <20181226184509.aoreae3ova3k2lz3@esperanza> On Wed, Dec 26, 2018 at 09:45:09PM +0300, Vladimir Davydov wrote: > On Sun, Dec 16, 2018 at 11:17:25PM +0300, Alexander Turenko wrote: > > Needed for #3276. > > --- > > src/lua/lua_iterator.h | 117 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 117 insertions(+) > > create mode 100644 src/lua/lua_iterator.h > > > > diff --git a/src/lua/lua_iterator.h b/src/lua/lua_iterator.h > > new file mode 100644 > > index 000000000..3e1a88c93 > > --- /dev/null > > +++ b/src/lua/lua_iterator.h > > @@ -0,0 +1,117 @@ > > +#ifndef TARANTOOL_LUA_LUA_ITERATOR_H_INCLUDED > > +#define TARANTOOL_LUA_LUA_ITERATOR_H_INCLUDED 1 > > Nit: we don't set an include guard macro to any particular value. > I removed this file now, but I fixed guard in merger.h. > > > > +/** > > + * This module contains helper functions to interact with a Lua > > + * iterator from C. > > + */ > > + > > +#include <lua.h> > > extern 'C' is missing. > The declarations was moved to utils.h, under extern 'C'. > > + > > +/** > > + * Holds iterator state (references to Lua objects). > > + */ > > +struct lua_iterator { > > I guess this structure as well as the functions below should have > luaL_ prefix. > The idea was to name the class 'lua iterator', but now I see it looks like namespace/prefix 'lua'. Replaced 'lua' with 'luaL'. > > + int gen; > > + int param; > > + int state; > > +}; > > + > > +/** > > + * Create a Lua iterator from {gen, param, state}. > > + */ > > +struct lua_iterator * > > +lua_iterator_new_fromtable(lua_State *L, int idx) > > If you include this header into two different source files, you'll get a > linker error. All functions defined in a header should be marked static > inline. Anyway, I don't think that defining these functions in a header > is a good idea - they are heavy and not supposed to be blazing fast. > Ouch, thanks. > May be, just move them to lua/util.c? Moved to src/lua/utils.[ch]. I feel 'one class is one module' approach as kinda more structured, but this is really small one. I wrapped this functions into {{{ }}} block instead. > > +/** > > + * Free all resources hold by the iterator. > > Nit: s/hold/held Fixed. > > > + */ > > +void lua_iterator_free(lua_State *L, struct lua_iterator *it) > > +{ > > + luaL_unref(L, LUA_REGISTRYINDEX, it->gen); > > + luaL_unref(L, LUA_REGISTRYINDEX, it->param); > > + luaL_unref(L, LUA_REGISTRYINDEX, it->state); > > + free(it); > > +} > > + > > +#endif /* TARANTOOL_LUA_LUA_ITERATOR_H_INCLUDED */
next prev parent reply other threads:[~2018-12-31 6:43 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-16 20:17 [PATCH 0/3] Merger Alexander Turenko 2018-12-16 20:17 ` [PATCH 1/3] Add luaT_iscallable with support of cdata metatype Alexander Turenko 2018-12-26 18:35 ` Vladimir Davydov 2018-12-28 1:46 ` Alexander Turenko 2018-12-28 8:00 ` Vladimir Davydov 2018-12-16 20:17 ` [PATCH 2/3] Add module to ease using Lua iterators from C Alexander Turenko 2018-12-26 18:45 ` Vladimir Davydov 2018-12-31 6:43 ` Alexander Turenko [this message] 2018-12-16 20:17 ` [PATCH 3/3] Add merger for tuple streams Alexander Turenko 2018-12-26 20:11 ` Vladimir Davydov 2019-01-09 21:36 ` Alexander Turenko 2018-12-18 12:16 ` [PATCH 0/3] Merger Alexander Turenko 2019-03-22 14:24 ` [tarantool-patches] [PATCH 0/3] lua: add key_def lua module Kirill Shcherbatov 2019-03-22 16:20 ` Alexander Turenko
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=20181231064355.fsfb7om3p7yqieqv@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 2/3] Add module to ease using Lua iterators from C' \ /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