From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 741AD469719 for ; Tue, 29 Sep 2020 08:52:50 +0300 (MSK) Date: Tue, 29 Sep 2020 08:53:01 +0300 From: Alexander Turenko Message-ID: <20200929055301.udvoq7j4n6fupit6@tkn_work_nb> References: <51700ea1fd13c5cbea49e3426551d1c16c1bda02.1600955781.git.tsafin@tarantool.org> <2958f26a-ef0a-4e13-70ec-b99b05cdb2b9@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2958f26a-ef0a-4e13-70ec-b99b05cdb2b9@tarantool.org> 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: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org > > +/** > > + * 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. > 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). 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. > If you use ibuf internally in the merger, what is wrong with > registering it in the merger, and storing a global variable of > uint32_t type, like we do inside tarantool executable? You will even > get the same type index, but won't need to carry small library > types into the public lua API. *check*, *is* helpers are usable and I would not be against exposing them: just to don't write the same code in different modules. However, if there are certain objections (not just 'you can implement it in your module'), it can be skipped because of simplicity. I think we should consider exposing things, which are useful for module writters (and C stored procedure writters): tarantool is the tool and I hope we all want it to be nice tool.