From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 B3EC1469719 for ; Thu, 8 Oct 2020 16:50:29 +0300 (MSK) From: "Timur Safin" References: <51700ea1fd13c5cbea49e3426551d1c16c1bda02.1600955781.git.tsafin@tarantool.org> <2958f26a-ef0a-4e13-70ec-b99b05cdb2b9@tarantool.org> <20200929055301.udvoq7j4n6fupit6@tkn_work_nb> <20201001030004.755sdofapdsuu77u@tkn_work_nb> In-Reply-To: <20201001030004.755sdofapdsuu77u@tkn_work_nb> Date: Thu, 8 Oct 2020 16:50:27 +0300 Message-ID: <149d01d69d79$facdc000$f0694000$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Tarantool-patches] [PATCH 2.X 6/7] module api: luaL_checkibuf & luaL_checkconstchar List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Alexander Turenko' , 'Vladislav Shpilevoy' , Aleksandr Lyapunov Cc: tarantool-patches@dev.tarantool.org : From: Alexander Turenko : Subject: Re: [Tarantool-patches] [PATCH 2.X 6/7] module api: : luaL_checkibuf & luaL_checkconstchar : : On Wed, Sep 30, 2020 at 01:25:15AM +0200, Vladislav Shpilevoy wrote: : > On 29.09.2020 07:53, Alexander Turenko wrote: : > >>> +/** : > >>> + * Check if a value on @a L stack by index @a idx is an ibuf : > >>> + * object. Both 'struct ibuf' and 'struct ibuf *' are accepted. : > >>> + * Returns NULL, if can't convert - not an ibuf object. : > >>> + */ : > >>> +struct ibuf * : > >>> +luaL_checkibuf(struct lua_State *L, int idx); : > >> : > >> 1. IMO 'check' is almost always worse than 'is'. Because you leave : > >> a user no choice but to use lua_cpcall if he does not want an : > >> exception. With 'is' he would be able to decide whether he wants to : > >> throw. The same for the method below. : > > : > > I personally prefer *is* Lua functions. It is good to have *is* : variants : > > in the public API aside of (or even instead of) *check* ones. I don't : > > insist, however. : > : > This is what I said. *is* is better than *check*. More freedom to decide : > what to do, when it returns false. : : I just agreed explicitly. : : > : > >> Also what exactly is the reason you need the ibuf checking for? : > >> Ibuf is not exposed as a part of tarantool binary. It is a part of : > >> small library. When we want to export parts of small from inside : > >> of the executable, we need to obfuscate the types and wrap the : > >> functions, because user's small library may be different from the : > >> executable's small. : > > : > > It seems, we really should expose non-opacue : rpos and : wpos : > > are used in merger (it would be too expensive to wrap them into : calls). : > > The module accepts ibuf created from Lua (tarantool's ), : so : > > linking with external small is not an option (unless we'll really care : > > about ABI). : > : > It seems you care about ABI a lot. So why do you stop now? If we don't : care : > about ABI that much, then why do we worry about box_key_part_def_t? : : 'unless we'll really care about ABI' was regarding the small ABI. Sorry, : now I see that it is not obvious from my wording. : : I'll repeat options as a list just in case: : : 1. Expose and () from the module API. : 2. Ensure that we can hold small's ibuf API / ABI (from several sides: : ability to extend it in future, static asserts, testing, no implicit : dependency on a particular toolchain behaviour) and use it directly. : : Both ways are okay for me. The second one is maybe better, but it is : more dangerous considering our current deadlines. : : I would highlight that just 'let's use small directly, unlikely it will : be changed' does not work in the long terms. So the second way is not : cheap, IMHO. Many things should be considered before we'll able to state : that, say, ibuf API / ABI is perfectly stable. I've discussed these things matter with Alexander Lyapunov, and despite his plans to introduce here "quite soon" (well in 4-6 month time frame) newer C++ connector facilities, which would allow to manipulate with raw buffers (like char*) in a more efficient manner, but he confirms the fact that he plans to supports binary compatibility in ibuf for indefinite time, and it's safe to use it externally today. And using small as git submodule is intended scenario. SashaL, do I put it correctly? Timur