Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH 2/3] Add module to ease using Lua iterators from C
Date: Wed, 26 Dec 2018 21:45:09 +0300	[thread overview]
Message-ID: <20181226184509.aoreae3ova3k2lz3@esperanza> (raw)
In-Reply-To: <84dde7baf7a3d9364d3199a6eb705c725589bad0.1544989900.git.alexander.turenko@tarantool.org>

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.

> +/*
> + * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + *    copyright notice, this list of conditions and the
> + *    following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + *    copyright notice, this list of conditions and the following
> + *    disclaimer in the documentation and/or other materials
> + *    provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY AUTHORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * AUTHORS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +/**
> + * This module contains helper functions to interact with a Lua
> + * iterator from C.
> + */
> +
> +#include <lua.h>

extern 'C' is missing.

> +
> +/**
> + * Holds iterator state (references to Lua objects).
> + */
> +struct lua_iterator {

I guess this structure as well as the functions below should have
luaL_ prefix.

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

May be, just move them to lua/util.c?

> +{
> +	struct lua_iterator *it = (struct lua_iterator *) malloc(
> +		sizeof(struct lua_iterator));
> +
> +	lua_rawgeti(L, idx, 1); /* Popped by luaL_ref(). */
> +	it->gen = luaL_ref(L, LUA_REGISTRYINDEX);
> +	lua_rawgeti(L, idx, 2); /* Popped by luaL_ref(). */
> +	it->param = luaL_ref(L, LUA_REGISTRYINDEX);
> +	lua_rawgeti(L, idx, 3); /* Popped by luaL_ref(). */
> +	it->state = luaL_ref(L, LUA_REGISTRYINDEX);
> +
> +	return it;
> +}
> +
> +/**
> + * Move iterator to the next value. Push values returned by
> + * gen(param, state) and return its count. Zero means no more
> + * results available.
> + */
> +int
> +lua_iterator_next(lua_State *L, struct lua_iterator *it)
> +{
> +	int frame_start = lua_gettop(L);
> +
> +	/* Call gen(param, state). */
> +	lua_rawgeti(L, LUA_REGISTRYINDEX, it->gen);
> +	lua_rawgeti(L, LUA_REGISTRYINDEX, it->param);
> +	lua_rawgeti(L, LUA_REGISTRYINDEX, it->state);
> +	lua_call(L, 2, LUA_MULTRET);
> +	int nresults = lua_gettop(L) - frame_start;
> +	if (nresults == 0) {
> +		luaL_error(L, "lua_iterator_next: gen(param, state) must "
> +			      "return at least one result");
> +		unreachable();
> +		return 0;
> +	}
> +
> +	/* The call above returns nil as the first result. */
> +	if (lua_isnil(L, frame_start + 1)) {
> +		lua_settop(L, frame_start);
> +		return 0;
> +	}
> +
> +	/* Save the first result to it->state. */
> +	luaL_unref(L, LUA_REGISTRYINDEX, it->state);
> +	lua_pushvalue(L, frame_start + 1); /* Popped by luaL_ref(). */
> +	it->state = luaL_ref(L, LUA_REGISTRYINDEX);
> +
> +	return nresults;
> +}
> +
> +/**
> + * Free all resources hold by the iterator.

Nit: s/hold/held

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

  reply	other threads:[~2018-12-26 18:45 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 [this message]
2018-12-31  6:43     ` Alexander Turenko
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=20181226184509.aoreae3ova3k2lz3@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --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