[tarantool-patches] Re: [PATCH v2 2/7] box: add method dump_lua to port
Imeev Mergen
imeevma at tarantool.org
Tue Nov 27 22:25:33 MSK 2018
Hi! Thank you for review and fixes! I squashed your fixes and
removed libs from port.c that were added in previous version.
New version below.
On 11/23/18 12:49 AM, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes! See my 5 comments below, fix
> at the end of the email and on the branch.
>
> Also, please, do not forget next time to answer to my comments
> inlined. Do not just send v2 without any answers.
>
> On 22/11/2018 22:10, imeevma at tarantool.org wrote:
>> New method dump_lua dumps saved in port tuples to Lua stack. It
>> will allow us to call this method without any other interaction
>> with port.
>>
>> Needed for #3505
>> ---
>> src/box/lua/call.c | 1 +
>> src/box/port.c | 22 ++++++++++++++++++++++
>> src/box/port.h | 12 ++++++++++++
>> 3 files changed, 35 insertions(+)
>>
>> diff --git a/src/box/port.c b/src/box/port.c
>> index 266cf3d..a65a32d 100644
>> --- a/src/box/port.c
>> +++ b/src/box/port.c
>> @@ -36,6 +36,8 @@
>> #include <small/mempool.h>
>> #include <fiber.h>
>> #include "errinj.h"
>> +#include "lua/utils.h"
>> +#include "lua/tuple.h"
>
> 1. src/box/ files should not depend in Lua. I moved
> implementation to misc.cc and put here extern declaration.
Squashed.
>
>> static struct mempool port_tuple_entry_pool;
>> @@ -121,6 +123,19 @@ port_tuple_dump_msgpack(struct port *base,
>> struct obuf *out)
>> return 1;
>> }
>> +static int
>
> 2. Why do you need to return number of dumped tuples? Dump_msgpack
> returns number of tuples since in iproto.cc it firstly reserves
> msgpack array header, then dumps port and then fills the reserved
> header. For Lua we do not need it.
Squashed.
>
>> +port_tuple_dump_lua(struct port *base, struct lua_State *L)
>> +{
>> + struct port_tuple *port = port_tuple(base);
>> + struct port_tuple_entry *pe;
>> + int i = 0;
>
> 3. Why did not you add here lua_createtable(L, port->size, 0);
> as it is done in misc.cc in the original implementation?
Squashed.
>
> 4. Why did not you replaced lbox_port_to_table with this new
> method?
>
> I did this points and it works.
Squashed.
>
>> + for (pe = port->first; pe != NULL; pe = pe->next) {
>> + luaT_pushtuple(L, pe->tuple);
>> + lua_rawseti(L, -2, ++i);
>> + }
>> + return port->size;
>> +}
>> +
>> void
>> port_destroy(struct port *port)
>> {
>> diff --git a/src/box/port.h b/src/box/port.h
>> index 882bb37..3bd83b0 100644
>> --- a/src/box/port.h
>> +++ b/src/box/port.h
>> @@ -185,6 +190,13 @@ int
>> port_dump_msgpack_16(struct port *port, struct obuf *out);
>> /**
>> + * Same as port_dump(), but use the legacy Tarantool 1.6
>> + * format.
>
> 5. This comment does not make sense to this function.
Squashed.
>
>> + */
>> +int
>> +port_dump_lua(struct port *port, struct lua_State *L);
>> +
>> +/**
>> * Dump a port content as a plain text into a buffer,
>> * allocated inside.
>> * @param port Port with data to dump.
>>
>
*New version:*
commit ff2fc3fd58dd99d3a98ad790c8e82363949cb3db
Author: Mergen Imeev <imeevma at gmail.com>
Date: Sat Nov 17 15:37:17 2018 +0300
box: add method dump_lua to port
New method dump_lua dumps saved in port tuples to Lua stack. It
will allow us to call this method without any other interaction
with port.
Needed for #3505
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 1f20426..52939ae 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -424,6 +424,7 @@ port_lua_dump_plain(struct port *port, uint32_t *size);
static const struct port_vtab port_lua_vtab = {
.dump_msgpack = port_lua_dump,
.dump_msgpack_16 = port_lua_dump_16,
+ .dump_lua = NULL,
.dump_plain = port_lua_dump_plain,
.destroy = port_lua_destroy,
};
diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index 8bd33ae..8de7401 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -56,23 +56,26 @@ lbox_encode_tuple_on_gc(lua_State *L, int idx,
size_t *p_len)
return (char *) region_join_xc(gc, *p_len);
}
-/* }}} */
-
-/** {{{ Lua/C implementation of index:select(): used only by Vinyl **/
-
-static inline void
-lbox_port_to_table(lua_State *L, struct port *port_base)
+/**
+ * Dump port_tuple content to Lua as a table. Used in box/port.c,
+ * but implemented here to eliminate port.c dependency on Lua.
+ */
+extern "C" void
+port_tuple_dump_lua(struct port *base, struct lua_State *L)
{
- struct port_tuple *port = port_tuple(port_base);
+ struct port_tuple *port = port_tuple(base);
lua_createtable(L, port->size, 0);
- struct port_tuple_entry *entry = port->first;
- for (int i = 0 ; i < port->size; i++) {
- luaT_pushtuple(L, entry->tuple);
- lua_rawseti(L, -2, i + 1);
- entry = entry->next;
+ struct port_tuple_entry *pe = port->first;
+ for (int i = 0; pe != NULL; pe = pe->next) {
+ luaT_pushtuple(L, pe->tuple);
+ lua_rawseti(L, -2, ++i);
}
}
+/* }}} */
+
+/** {{{ Lua/C implementation of index:select(): used only by Vinyl **/
+
static int
lbox_select(lua_State *L)
{
@@ -106,7 +109,7 @@ lbox_select(lua_State *L)
* table always crashed the first (can't be fixed with pcall).
* https://github.com/tarantool/tarantool/issues/1182
*/
- lbox_port_to_table(L, &port);
+ port_dump_lua(&port, L);
port_destroy(&port);
return 1; /* lua table with tuples */
}
diff --git a/src/box/port.c b/src/box/port.c
index 266cf3d..853d24c 100644
--- a/src/box/port.c
+++ b/src/box/port.c
@@ -121,6 +121,9 @@ port_tuple_dump_msgpack(struct port *base, struct
obuf *out)
return 1;
}
+extern void
+port_tuple_dump_lua(struct port *base, struct lua_State *L);
+
void
port_destroy(struct port *port)
{
@@ -139,6 +142,12 @@ port_dump_msgpack_16(struct port *port, struct obuf
*out)
return port->vtab->dump_msgpack_16(port, out);
}
+void
+port_dump_lua(struct port *port, struct lua_State *L)
+{
+ port->vtab->dump_lua(port, L);
+}
+
const char *
port_dump_plain(struct port *port, uint32_t *size)
{
@@ -161,6 +170,7 @@ port_free(void)
const struct port_vtab port_tuple_vtab = {
.dump_msgpack = port_tuple_dump_msgpack,
.dump_msgpack_16 = port_tuple_dump_msgpack_16,
+ .dump_lua = port_tuple_dump_lua,
.dump_plain = NULL,
.destroy = port_tuple_destroy,
};
diff --git a/src/box/port.h b/src/box/port.h
index 882bb37..751e44e 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -77,6 +77,8 @@ struct port_vtab {
* 1.6 format.
*/
int (*dump_msgpack_16)(struct port *port, struct obuf *out);
+ /** Dump the content of a port to Lua stack. */
+ void (*dump_lua)(struct port *port, struct lua_State *L);
/**
* Dump a port content as a plain text into a buffer,
* allocated inside.
@@ -184,6 +186,10 @@ port_dump_msgpack(struct port *port, struct obuf *out);
int
port_dump_msgpack_16(struct port *port, struct obuf *out);
+/** Dump port content to Lua stack. */
+void
+port_dump_lua(struct port *port, struct lua_State *L);
+
/**
* Dump a port content as a plain text into a buffer,
* allocated inside.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20181127/5bc1a8ec/attachment.html>
More information about the Tarantool-patches
mailing list