From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2966A469719 for ; Fri, 16 Oct 2020 00:07:07 +0300 (MSK) From: "Timur Safin" References: <20201014035039.glx2veliqncoyp5z@tkn_work_nb> In-Reply-To: <20201014035039.glx2veliqncoyp5z@tkn_work_nb> Date: Fri, 16 Oct 2020 00:07:05 +0300 Message-ID: <053001d6a337$22f88750$68e995f0$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: ru Subject: Re: [Tarantool-patches] [PATCH 2.X v4 3/4] module api: introduced luaT_toibuf instead of luaL_checkibuf List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Alexander Turenko' Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Renamed function in the test, and slightly reworded commit message: commit e0e1421c660dcae8bcced91f392a2d1093ecf30e Author: Timur Safin 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 =3D luaT_toibuf(L, -1); + lua_pushboolean(L, buf !=3D 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 =3D require('ffi') + local buffer =3D require('buffer') + + local bufalloc =3D buffer.static_alloc("char", 128) + local ibuf =3D buffer.ibuf() + local pbuf =3D 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 : Subject: Re: [PATCH 2.X v4 3/4] module api: introduced luaT_toibuf = instead : of luaL_checkibuf :=20 : > module api: introduced luaT_toibuf instead of luaL_checkibuf :=20 : 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? :=20 : BTW, imperative mood ('introduce') is suggested for a commit message : header (just header, not the entire body). :=20 : (I would name it 'module api/lua: expose luaT_toibuf()' if you want my : opinion on this essential topic.) :=20 : > * made `luaL_checkibuf` public; : > * renamed it to `luaT_toibuf` to follow naming convention : > (it's not raising error, and is casting to ibuf type). :=20 : 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. :=20 : I don't push you to anything, just noted that I'm a bit confused here = as : a reader. :=20 : > +static int : > +test_checkibuf(lua_State *L) : > +{ : > + struct ibuf *buf; : > + buf =3D luaT_toibuf(L, -1); : > + lua_pushboolean(L, buf !=3D NULL); : > + return 1; : > +} :=20 : I don't bother enough about the test to insist on it now, but it looks : just as inconsistency. Why not test_toibuf()? :=20 : > +local function test_buffers(test, module) : > + test:plan(9) : > + local ffi =3D require('ffi') : > + local buffer =3D require('buffer') : > + : > + local bufalloc =3D buffer.static_alloc("char", 128) : > + local ibuf =3D buffer.ibuf() : > + local pbuf =3D 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') :=20 : And here too: why 'checkibuf of nil'? The subject of the test is : luaT_isibuf().