Hi! Thanks for the fixes! See my 5 comments below, fixSquashed.
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@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.
+ */
+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@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.