Re: [Tarantool-patches] [PATCH 2.X v4 3/4] module api: introduced luaT_toibuf instead of luaL_checkibuf

["Timur Safin" <tsafin@tarantool.org>]

Renamed function in the test, and slightly reworded commit message:

commit e0e1421c660dcae8bcced91f392a2d1093ecf30e
Author: Timur Safin <tsafin@tarantool.org>
Date:   Sun Oct 11 16:08:02 2020 +0300

    module api: luaT_toibuf instead of luaL_checkibuf

    * made `luaL_checkibuf` public and then renamed to
      `luaT_toibuf` to follow naming convention
      (it's not raising error, simply returning ibuf
      structure).

    Part of #5384

...
diff --git a/test/app-tap/module_api.c b/test/app-tap/module_api.c
index 17048081f..1c5723693 100644
--- a/test/app-tap/module_api.c
+++ b/test/app-tap/module_api.c
@@ -155,6 +155,15 @@ test_checkint64(lua_State *L)
        return 1;
 }

+static int
+test_toibuf(lua_State *L)
+{
+       struct ibuf *buf;
+       buf = luaT_toibuf(L, -1);
+       lua_pushboolean(L, buf != NULL);
+       return 1;
+}
+
 static int
 test_touint64(lua_State *L)
 {
@@ -1242,6 +1251,7 @@ luaopen_module_api(lua_State *L)
                {"test_pushint64", test_pushint64 },
                {"test_checkuint64", test_checkuint64 },
                {"test_checkint64", test_checkint64 },
+               {"toibuf", test_toibuf},
                {"test_touint64", test_touint64 },
                {"test_toint64", test_toint64 },
                {"test_fiber", test_fiber },
diff --git a/test/app-tap/module_api.test.lua b/test/app-tap/module_api.test.lua
index ab5c89c8c..c1aff6797 100755
--- a/test/app-tap/module_api.test.lua
+++ b/test/app-tap/module_api.test.lua
@@ -38,6 +38,26 @@ local function test_pushcdata(test, module)
     test:is(gc_counter, 1, 'pushcdata gc')
 end

+local function test_buffers(test, module)
+    test:plan(9)
+    local ffi = require('ffi')
+    local buffer = require('buffer')
+
+    local bufalloc = buffer.static_alloc("char", 128)
+    local ibuf = buffer.ibuf()
+    local pbuf = ibuf:alloc(128)
+
+    test:ok(not module.toibuf(nil), 'toibuf of nil')
+    test:ok(not module.toibuf({}), 'toibuf of {}')
+    test:ok(not module.toibuf(1LL), 'toibuf of 1LL')
+    test:ok(not module.toibuf(box.NULL), 'toibuf of box.NULL')
+    test:ok(not module.toibuf(buffer.reg1), 'toibuf of reg1')
+    test:ok(not module.toibuf(bufalloc), 'toibuf of allocated buffer')
+    test:ok(module.toibuf(buffer.IBUF_SHARED), "toibuf of ibuf*")
+    test:ok(module.toibuf(ibuf), 'toibuf of ibuf')
+    test:ok(not module.toibuf(pbuf), 'toibuf of pointer to ibuf data')
+end
+
 local function test_tuple_validate(test, module)
     test:plan(12)

: From: Alexander Turenko <alexander.turenko@tarantool.org>
: Subject: Re: [PATCH 2.X v4 3/4] module api: introduced luaT_toibuf instead
: of luaL_checkibuf
: 
: > module api: introduced luaT_toibuf instead of luaL_checkibuf
: 
: We usually try to fit into 50 symbols. Okay, sometimes we overrun it a
: bit (and I was one of persons who asked to don't make it hard limit),
: where it is hard to give a short description, but it is not the case,
: right?
: 
: BTW, imperative mood ('introduce') is suggested for a commit message
: header (just header, not the entire body).
: 
: (I would name it 'module api/lua: expose luaT_toibuf()' if you want my
: opinion on this essential topic.)
: 
: > * made `luaL_checkibuf` public;
: > * renamed it to `luaT_toibuf` to follow naming convention
: >   (it's not raising error, and is casting to ibuf type).
: 
: Did you mean 'returns a pointer to' by 'is casting to'? It tooks some
: time to get the idea. Hmm, but lua_check<...> also returns a pointer.
: 
: I don't push you to anything, just noted that I'm a bit confused here as
: a reader.
: 
: > +static int
: > +test_checkibuf(lua_State *L)
: > +{
: > +	struct ibuf *buf;
: > +	buf = luaT_toibuf(L, -1);
: > +	lua_pushboolean(L, buf != NULL);
: > +	return 1;
: > +}
: 
: I don't bother enough about the test to insist on it now, but it looks
: just as inconsistency. Why not test_toibuf()?
: 
: > +local function test_buffers(test, module)
: > +    test:plan(9)
: > +    local ffi = require('ffi')
: > +    local buffer = require('buffer')
: > +
: > +    local bufalloc = buffer.static_alloc("char", 128)
: > +    local ibuf = buffer.ibuf()
: > +    local pbuf = ibuf:alloc(128)
: > +
: > +    test:ok(not module.checkibuf(nil), 'checkibuf of nil')
: > +    test:ok(not module.checkibuf({}), 'checkibuf of {}')
: > +    test:ok(not module.checkibuf(1LL), 'checkibuf of 1LL')
: > +    test:ok(not module.checkibuf(box.NULL), 'checkibuf of box.NULL')
: > +    test:ok(not module.checkibuf(buffer.reg1), 'checkibuf of reg1')
: > +    test:ok(not module.checkibuf(bufalloc), 'checkibuf of allocated
: buffer')
: > +    test:ok(module.checkibuf(buffer.IBUF_SHARED), "checkibuf of ibuf*")
: > +    test:ok(module.checkibuf(ibuf), 'checkibuf of ibuf')
: > +    test:ok(not module.checkibuf(pbuf), 'checkibuf of pointer to ibuf
: data')
: 
: And here too: why 'checkibuf of nil'? The subject of the test is
: luaT_isibuf().