From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 240FA44643D for ; Tue, 29 Sep 2020 01:21:25 +0300 (MSK) References: <51700ea1fd13c5cbea49e3426551d1c16c1bda02.1600955781.git.tsafin@tarantool.org> From: Vladislav Shpilevoy Message-ID: <2958f26a-ef0a-4e13-70ec-b99b05cdb2b9@tarantool.org> Date: Tue, 29 Sep 2020 00:21:23 +0200 MIME-Version: 1.0 In-Reply-To: <51700ea1fd13c5cbea49e3426551d1c16c1bda02.1600955781.git.tsafin@tarantool.org> 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: Timur Safin , alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! See 2 comments below. > diff --git a/src/lua/utils.h b/src/lua/utils.h > index da0140076..6b10d2755 100644 > --- a/src/lua/utils.h > +++ b/src/lua/utils.h > @@ -589,6 +589,23 @@ luaT_temp_luastate(int *coro_ref, int *top); > void > luaT_release_temp_luastate(struct lua_State *L, int coro_ref, int top); > > +/** > + * 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. 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. 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 if a value on @a L stack by index @a idx is pointer at > + * char or const char. '(char *)NULL' is also considered a valid > + * char pointer. > + */ > +int > +luaL_checkconstchar(struct lua_State *L, int idx, const char **res, > + uint32_t *cdata_type_p); 2. What is 'res'? What is 'cdata_type_p'? > + > /** \endcond public */ > > /**