From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 5F24C469719 for ; Wed, 30 Sep 2020 02:25:17 +0300 (MSK) References: <51700ea1fd13c5cbea49e3426551d1c16c1bda02.1600955781.git.tsafin@tarantool.org> <2958f26a-ef0a-4e13-70ec-b99b05cdb2b9@tarantool.org> <20200929055301.udvoq7j4n6fupit6@tkn_work_nb> From: Vladislav Shpilevoy Message-ID: Date: Wed, 30 Sep 2020 01:25:15 +0200 MIME-Version: 1.0 In-Reply-To: <20200929055301.udvoq7j4n6fupit6@tkn_work_nb> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 Cc: tarantool-patches@dev.tarantool.org 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. >> 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? I am ok with box_ibuf_t, of any kind. Then we could eventually legally expose tarantool_lua_ibuf, as at least for single alloc+free pairs it is better than region (a tiny tiny better). > Maybe we can alias with if we'll add static > asserts around the structure size and offset of fields? And expose > box_ibuf_*(), of course. Also looks fine. However returning to the topic, it seems not related to the Lua type ids. You don't need ibuf definition to get its type ID in C. All is done in luaL_ctypeid function taking a string anyway. luaL_ctypeid(L, "struct ibuf *"); luaL_ctypeid(L, "struct ibuf"); So why does Timur need that method exported? luaL_checkibuf is a trival function, which can be implemented by luaL_iscdata() + getting ibuf type id using luaL_ctypeid() and comparing result with the cdata type id.