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