[PATCH 0/2] Move info to src/ for SWIM

[PATCH 0/2] Move info to src/ for SWIM

[Vladislav Shpilevoy]

First commit has enough explanation of this patchset but nonetheless
it is formally required so there it is.

SWIM is a forthcoming independent module which is going to live in
src/lib. And it will have a method swim.info() which dumps SWIM
state to Lua.

But SWIM will be written on C and should not depend on Lua. To
resolve the problem the patchset moves src/box/info to src/info
to make it usable in SWIM.

src/box/info is an interface which allows to dump something to Lua
without getting in touch with Lua API.

Second commit in the patchset removes some false comments about info
interface stating that it throws C++ exceptions.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/swim-preparation-info
Issue: https://github.com/tarantool/tarantool/issues/3234

Vladislav Shpilevoy (2):
  box: move info_handler interface into src/info
  info: remove false comments from src/info.h

 src/CMakeLists.txt   |   1 +
 src/box/lua/index.c  |   4 +-
 src/box/lua/info.c   |  78 +------------------------------
 src/box/lua/sql.c    |   4 +-
 src/box/lua/stat.c   |   4 +-
 src/box/sql.c        |   2 +-
 src/{box => }/info.h |  13 ++----
 src/lua/info.c       | 107 +++++++++++++++++++++++++++++++++++++++++++
 src/lua/info.h       |  49 ++++++++++++++++++++
 9 files changed, 169 insertions(+), 93 deletions(-)
 rename src/{box => }/info.h (91%)
 create mode 100644 src/lua/info.c
 create mode 100644 src/lua/info.h

-- 
2.17.2 (Apple Git-113)

[PATCH 1/2] box: move info_handler interface into src/info

[Vladislav Shpilevoy]

Box/info.h defines info_handler interface with a set
of virtual functions. It allows to hide Lua from code
not depending on this language, and is used in things
like index:info(), box.info() to build Lua table with
some info. But it does not depend on box/ so move it
to src/.

Also, this API is needed for the forthcoming SWIM
module which is going to be placed into src/lib and
needs info to dump its state to Lua from C without
strict Lua dependency.

Needed for #3234
---
 src/CMakeLists.txt   |   1 +
 src/box/lua/index.c  |   4 +-
 src/box/lua/info.c   |  78 +------------------------------
 src/box/lua/sql.c    |   4 +-
 src/box/lua/stat.c   |   4 +-
 src/box/sql.c        |   2 +-
 src/{box => }/info.h |   6 +--
 src/lua/info.c       | 107 +++++++++++++++++++++++++++++++++++++++++++
 src/lua/info.h       |  49 ++++++++++++++++++++
 9 files changed, 169 insertions(+), 86 deletions(-)
 rename src/{box => }/info.h (97%)
 create mode 100644 src/lua/info.c
 create mode 100644 src/lua/info.h

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index e004f2154..7d0734f55 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -179,6 +179,7 @@ set (server_sources
      lua/crypto.c
      lua/httpc.c
      lua/utf8.c
+     lua/info.c
      ${lua_sources}
      ${PROJECT_SOURCE_DIR}/third_party/lua-yaml/lyaml.cc
      ${PROJECT_SOURCE_DIR}/third_party/lua-yaml/b64.c
diff --git a/src/box/lua/index.c b/src/box/lua/index.c
index ef89c397d..6265c044a 100644
--- a/src/box/lua/index.c
+++ b/src/box/lua/index.c
@@ -30,10 +30,10 @@
  */
 #include "box/lua/index.h"
 #include "lua/utils.h"
+#include "lua/info.h"
+#include <info.h>
 #include "box/box.h"
 #include "box/index.h"
-#include "box/info.h"
-#include "box/lua/info.h"
 #include "box/lua/tuple.h"
 #include "box/lua/misc.h" /* lbox_encode_tuple_on_gc() */
 
diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index 03b953288..f94121e61 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -32,7 +32,7 @@
 #define _GNU_SOURCE
 #endif
 
-#include "box/lua/info.h"
+#include "lua/info.h"
 
 #include <ctype.h> /* tolower() */
 
@@ -45,7 +45,7 @@
 #include "box/iproto.h"
 #include "box/wal.h"
 #include "box/replication.h"
-#include "box/info.h"
+#include <info.h>
 #include "box/gc.h"
 #include "box/engine.h"
 #include "box/vinyl.h"
@@ -448,80 +448,6 @@ lbox_info_gc(struct lua_State *L)
 	return 1;
 }
 
-static void
-luaT_info_begin(struct info_handler *info)
-{
-	lua_State *L = (lua_State *) info->ctx;
-	lua_newtable(L);
-}
-
-static void
-luaT_info_end(struct info_handler *info)
-{
-	(void) info;
-}
-
-static void
-luaT_info_begin_table(struct info_handler *info, const char *key)
-{
-	lua_State *L = (lua_State *) info->ctx;
-	lua_pushstring(L, key);
-	lua_newtable(L);
-}
-
-static void
-luaT_info_end_table(struct info_handler *info)
-{
-	lua_State *L = (lua_State *) info->ctx;
-	lua_settable(L, -3);
-}
-
-static void
-luaT_info_append_double(struct info_handler *info,
-			const char *key, double value)
-{
-	lua_State *L = (lua_State *) info->ctx;
-	lua_pushstring(L, key);
-	lua_pushnumber(L, value);
-	lua_settable(L, -3);
-}
-
-static void
-luaT_info_append_int(struct info_handler *info, const char *key,
-		     int64_t value)
-{
-	lua_State *L = (lua_State *) info->ctx;
-	lua_pushstring(L, key);
-	luaL_pushint64(L, value);
-	lua_settable(L, -3);
-}
-
-static void
-luaT_info_append_str(struct info_handler *info, const char *key,
-		     const char *value)
-{
-	lua_State *L = (lua_State *) info->ctx;
-	lua_pushstring(L, key);
-	lua_pushstring(L, value);
-	lua_settable(L, -3);
-}
-
-void
-luaT_info_handler_create(struct info_handler *h, struct lua_State *L)
-{
-	static struct info_handler_vtab lua_vtab = {
-		.begin = luaT_info_begin,
-		.end = luaT_info_end,
-		.begin_table = luaT_info_begin_table,
-		.end_table = luaT_info_end_table,
-		.append_int = luaT_info_append_int,
-		.append_str = luaT_info_append_str,
-		.append_double = luaT_info_append_double
-	};
-	h->vtab = &lua_vtab;
-	h->ctx = L;
-}
-
 static int
 lbox_info_vinyl_call(struct lua_State *L)
 {
diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c
index 17e269423..692366607 100644
--- a/src/box/lua/sql.c
+++ b/src/box/lua/sql.c
@@ -3,9 +3,9 @@
 #include "lua/msgpack.h"
 
 #include "box/sql/sqliteInt.h"
-#include "box/info.h"
+#include <info.h>
+#include "lua/info.h"
 #include "lua/utils.h"
-#include "info.h"
 
 static void
 lua_push_column_names(struct lua_State *L, struct sqlite3_stmt *stmt)
diff --git a/src/box/lua/stat.c b/src/box/lua/stat.c
index 83a5b64e6..3fce81f61 100644
--- a/src/box/lua/stat.c
+++ b/src/box/lua/stat.c
@@ -42,8 +42,8 @@
 #include "box/iproto.h"
 #include "box/engine.h"
 #include "box/vinyl.h"
-#include "box/info.h"
-#include "box/lua/info.h"
+#include <info.h>
+#include "lua/info.h"
 #include "lua/utils.h"
 
 extern struct rmean *rmean_box;
diff --git a/src/box/sql.c b/src/box/sql.c
index c3418008c..1fcd3e809 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -36,7 +36,7 @@
 #include "sql/vdbeInt.h"
 
 #include "index.h"
-#include "info.h"
+#include <info.h>
 #include "schema.h"
 #include "box.h"
 #include "txn.h"
diff --git a/src/box/info.h b/src/info.h
similarity index 97%
rename from src/box/info.h
rename to src/info.h
index df82becd5..0198c31d4 100644
--- a/src/box/info.h
+++ b/src/info.h
@@ -1,5 +1,5 @@
-#ifndef INCLUDES_TARANTOOL_BOX_INFO_H
-#define INCLUDES_TARANTOOL_BOX_INFO_H
+#ifndef INCLUDES_TARANTOOL_INFO_H
+#define INCLUDES_TARANTOOL_INFO_H
 /*
  * Copyright 2010-2017, Tarantool AUTHORS, please see AUTHORS file.
  *
@@ -195,4 +195,4 @@ info_table_end(struct info_handler *info)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
 
-#endif /* INCLUDES_TARANTOOL_BOX_INFO_H */
+#endif /* INCLUDES_TARANTOOL_INFO_H */
diff --git a/src/lua/info.c b/src/lua/info.c
new file mode 100644
index 000000000..5ccbc611c
--- /dev/null
+++ b/src/lua/info.c
@@ -0,0 +1,107 @@
+/*
+ * 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 <COPYRIGHT HOLDER> ``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
+ * <COPYRIGHT HOLDER> 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.
+ */
+#include "lua/info.h"
+#include <info.h>
+#include "lua/utils.h"
+
+static void
+luaT_info_begin(struct info_handler *info)
+{
+	lua_State *L = (lua_State *) info->ctx;
+	lua_newtable(L);
+}
+
+static void
+luaT_info_end(struct info_handler *info)
+{
+	(void) info;
+}
+
+static void
+luaT_info_begin_table(struct info_handler *info, const char *key)
+{
+	lua_State *L = (lua_State *) info->ctx;
+	lua_pushstring(L, key);
+	lua_newtable(L);
+}
+
+static void
+luaT_info_end_table(struct info_handler *info)
+{
+	lua_State *L = (lua_State *) info->ctx;
+	lua_settable(L, -3);
+}
+
+static void
+luaT_info_append_double(struct info_handler *info,
+			const char *key, double value)
+{
+	lua_State *L = (lua_State *) info->ctx;
+	lua_pushstring(L, key);
+	lua_pushnumber(L, value);
+	lua_settable(L, -3);
+}
+
+static void
+luaT_info_append_int(struct info_handler *info, const char *key,
+		     int64_t value)
+{
+	lua_State *L = (lua_State *) info->ctx;
+	lua_pushstring(L, key);
+	luaL_pushint64(L, value);
+	lua_settable(L, -3);
+}
+
+static void
+luaT_info_append_str(struct info_handler *info, const char *key,
+		   const char *value)
+{
+	lua_State *L = (lua_State *) info->ctx;
+	lua_pushstring(L, key);
+	lua_pushstring(L, value);
+	lua_settable(L, -3);
+}
+
+void
+luaT_info_handler_create(struct info_handler *h, struct lua_State *L)
+{
+	static struct info_handler_vtab lua_vtab = {
+		.begin = luaT_info_begin,
+		.end = luaT_info_end,
+		.begin_table = luaT_info_begin_table,
+		.end_table = luaT_info_end_table,
+		.append_int = luaT_info_append_int,
+		.append_str = luaT_info_append_str,
+		.append_double = luaT_info_append_double
+	};
+	h->vtab = &lua_vtab;
+	h->ctx = L;
+}
diff --git a/src/lua/info.h b/src/lua/info.h
new file mode 100644
index 000000000..712d0f78d
--- /dev/null
+++ b/src/lua/info.h
@@ -0,0 +1,49 @@
+#ifndef INCLUDES_TARANTOOL_LUA_INFO_H
+#define INCLUDES_TARANTOOL_LUA_INFO_H
+
+/*
+ * Copyright 2010-2016, 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 <COPYRIGHT HOLDER> ``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
+ * <COPYRIGHT HOLDER> 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.
+ */
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct lua_State;
+struct info_handler;
+
+void
+luaT_info_handler_create(struct info_handler *h, struct lua_State *L);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
+#endif /* INCLUDES_TARANTOOL_LUA_INFO_H */
-- 
2.17.2 (Apple Git-113)

Re: [PATCH 1/2] box: move info_handler interface into src/info

[Vladimir Davydov]

On Tue, Dec 04, 2018 at 12:47:05AM +0300, Vladislav Shpilevoy wrote:
> +void
> +luaT_info_handler_create(struct info_handler *h, struct lua_State *L);

You forgot to remove luaT_info_handler declaration from box/lua/info.h.
I removed it by myself as well as the pointless _GNU_SOURCE definition
and pushed the patch to 2.1.

[PATCH 2/2] info: remove false comments from src/info.h

[Vladislav Shpilevoy]

---
 src/info.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/src/info.h b/src/info.h
index 0198c31d4..cafa425e9 100644
--- a/src/info.h
+++ b/src/info.h
@@ -103,7 +103,6 @@ struct info_handler {
 /**
  * Starts a new document and creates root-level associative array.
  * @param info box.info() adapter.
- * @throws C++ exception on OOM, see info.h comments.
  * @pre must be called once before any other functions.
  */
 static inline void
@@ -115,7 +114,6 @@ info_begin(struct info_handler *info)
 /**
  * Finishes the document and closes root-level associative array.
  * @param info box.info() adapter.
- * @throws C++ exception on OOM, see info.h comments.
  * @pre must be called at the end.
  */
 static inline void
@@ -129,7 +127,6 @@ info_end(struct info_handler *info)
  * @param info box.info() adapter.
  * @param key key.
  * @param value value.
- * @throws C++ exception on OOM, see info.h comments.
  * @pre associative array is started.
  */
 static inline void
@@ -144,7 +141,6 @@ info_append_int(struct info_handler *info, const char *key, int64_t value)
  * @param info box.info() adapter.
  * @param key key.
  * @param value value.
- * @throws C++ exception on OOM, see info.h comments.
  */
 static inline void
 info_append_str(struct info_handler *info, const char *key,
@@ -159,7 +155,6 @@ info_append_str(struct info_handler *info, const char *key,
  * @param info box.info() adapter.
  * @param key key.
  * @param value value.
- * @throws C++ exception on OOM, see info.h comments.
  */
 static inline void
 info_append_double(struct info_handler *info, const char *key,
@@ -172,7 +167,6 @@ info_append_double(struct info_handler *info, const char *key,
  * Associates a new associative array with @a key.
  * @param info box.info() adapter.
  * @param key key.
- * @throws C++ exception on OOM, see info.h comments.
  */
 static inline void
 info_table_begin(struct info_handler *info, const char *key)
@@ -183,7 +177,6 @@ info_table_begin(struct info_handler *info, const char *key)
 /*
  * Finishes the current active associative array.
  * @param info box.info() adapter
- * @throws C++ exception on OOM, see info.h comments.
  */
 static inline void
 info_table_end(struct info_handler *info)
-- 
2.17.2 (Apple Git-113)

Re: [PATCH 2/2] info: remove false comments from src/info.h

[Vladimir Davydov]

On Tue, Dec 04, 2018 at 12:47:06AM +0300, Vladislav Shpilevoy wrote:
> ---
>  src/info.h | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/src/info.h b/src/info.h
> index 0198c31d4..cafa425e9 100644
> --- a/src/info.h
> +++ b/src/info.h
> @@ -103,7 +103,6 @@ struct info_handler {
>  /**
>   * Starts a new document and creates root-level associative array.
>   * @param info box.info() adapter.
> - * @throws C++ exception on OOM, see info.h comments.
>   * @pre must be called once before any other functions.

These comments are valid - info handler does throw an exception on OOM,
and there's a comment in src/info.h mentioning that. So I dropped this
patch.

Re: [PATCH 2/2] info: remove false comments from src/info.h

[Vladislav Shpilevoy]

On 04/12/2018 12:02, Vladimir Davydov wrote:
> On Tue, Dec 04, 2018 at 12:47:06AM +0300, Vladislav Shpilevoy wrote:
>> ---
>>   src/info.h | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/src/info.h b/src/info.h
>> index 0198c31d4..cafa425e9 100644
>> --- a/src/info.h
>> +++ b/src/info.h
>> @@ -103,7 +103,6 @@ struct info_handler {
>>   /**
>>    * Starts a new document and creates root-level associative array.
>>    * @param info box.info() adapter.
>> - * @throws C++ exception on OOM, see info.h comments.
>>    * @pre must be called once before any other functions.
> 
> These comments are valid - info handler does throw an exception on OOM,
> and there's a comment in src/info.h mentioning that. So I dropped this
> patch.
> 

Sorry, I though you forgot about it and pushed it by myself.

Also, it does not throw C++ exceptions, even on OOM.

Re: [PATCH 2/2] info: remove false comments from src/info.h

[Vladimir Davydov]

On Tue, Dec 04, 2018 at 12:11:09PM +0300, Vladislav Shpilevoy wrote:
> 
> 
> On 04/12/2018 12:02, Vladimir Davydov wrote:
> > On Tue, Dec 04, 2018 at 12:47:06AM +0300, Vladislav Shpilevoy wrote:
> > > ---
> > >   src/info.h | 7 -------
> > >   1 file changed, 7 deletions(-)
> > > 
> > > diff --git a/src/info.h b/src/info.h
> > > index 0198c31d4..cafa425e9 100644
> > > --- a/src/info.h
> > > +++ b/src/info.h
> > > @@ -103,7 +103,6 @@ struct info_handler {
> > >   /**
> > >    * Starts a new document and creates root-level associative array.
> > >    * @param info box.info() adapter.
> > > - * @throws C++ exception on OOM, see info.h comments.
> > >    * @pre must be called once before any other functions.
> > 
> > These comments are valid - info handler does throw an exception on OOM,
> > and there's a comment in src/info.h mentioning that. So I dropped this
> > patch.
> > 
> 
> Sorry, I though you forgot about it and pushed it by myself.

I don't usually forget anything, and in case I do, you can ping me on
Telegram or email rather than making implicit assumptions.

> 
> Also, it does not throw C++ exceptions, even on OOM.

This is the way this API was initially desgined: it may throw a Lua
exception or a C++ exception, depending on the implementation. Roman
added those comments intentionally.

I guess we'd remove them anyway when we made Tarantool panic on any
malloc() failure.

Re: [PATCH 2/2] info: remove false comments from src/info.h

[Vladislav Shpilevoy]

On 04/12/2018 12:46, Vladimir Davydov wrote:
> On Tue, Dec 04, 2018 at 12:11:09PM +0300, Vladislav Shpilevoy wrote:
>>
>>
>> On 04/12/2018 12:02, Vladimir Davydov wrote:
>>> On Tue, Dec 04, 2018 at 12:47:06AM +0300, Vladislav Shpilevoy wrote:
>>>> ---
>>>>    src/info.h | 7 -------
>>>>    1 file changed, 7 deletions(-)
>>>>
>>>> diff --git a/src/info.h b/src/info.h
>>>> index 0198c31d4..cafa425e9 100644
>>>> --- a/src/info.h
>>>> +++ b/src/info.h
>>>> @@ -103,7 +103,6 @@ struct info_handler {
>>>>    /**
>>>>     * Starts a new document and creates root-level associative array.
>>>>     * @param info box.info() adapter.
>>>> - * @throws C++ exception on OOM, see info.h comments.
>>>>     * @pre must be called once before any other functions.
>>>
>>> These comments are valid - info handler does throw an exception on OOM,
>>> and there's a comment in src/info.h mentioning that. So I dropped this
>>> patch.
>>>
>>
>> Sorry, I though you forgot about it and pushed it by myself.
> 
> I don't usually forget anything, and in case I do, you can ping me on
> Telegram or email rather than making implicit assumptions.

Understood. Sorry again.

> 
>>
>> Also, it does not throw C++ exceptions, even on OOM.
> 
> This is the way this API was initially desgined: it may throw a Lua
> exception or a C++ exception, depending on the implementation. Roman
> added those comments intentionally.

Yes, I saw git blame and Roman comments. But Lua error != C++ exception.
It can not be caught by try-catch block.

> 
> I guess we'd remove them anyway when we made Tarantool panic on any
> malloc() failure.

Ugum.