From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 64ADD4696C3 for ; Wed, 1 Apr 2020 18:35:33 +0300 (MSK) Date: Wed, 1 Apr 2020 18:35:35 +0300 From: Alexander Turenko Message-ID: <20200401153535.uk7olbxabip3os52@tkn_work_nb> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 0/6] Extending error functionality List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy We discussed several questions around the API and the implementation yesterday with Leonid and Vlad. Vlad summarized the API points in the issue (see [1]). I want to summarize implementation details we more-or-less agreed on. We have two places, where encoding and decoding is performed to/from msgpack: they are src/lua/{msgpack.c,msgpackffi.lua}. I propose to add two mappings to msgpack.c: | /* | * Encode an object on @an idx slot on the Lua stack into | * @a buf, promote @a buf pointer on written bytes amount. | * | * Don't write over @an end pointer (inclusive). | */ | typedef void (*encode_ext_f)(struct lua_State *L, int idx, char **buf, | char *end); | | /* | * Decode msgpack from @buf, push it onto the Lua stack. | * | * Promote @a buf on amount of read bytes. | * | * Don't read over @an end (inclusive). | * | */ | typedef void (*decode_ext_f)(struct lua_State *L, char **buf, char *end); | | /* cdata type id -> encode function */ | static mh_encode_ext; | | /* msgpack extension type -> decode function */ | static mh_decode_ext; | | /* Register encode function for a cdata type. */ | void | register_encode_ext(int ctypeid, encode_ext_f f); | | /* Register decode function for an extension type. */ | void | register_decode_ext(int mp_ext_type, decode_ext_f f); So box will add its own encode / decode functions for box.error objects on box initialization (at tarantool loading, not at box.cfg()). The problem we'll solve here is that src/lua/msgpack.c should not be statically linked with src/box. The similar change should be made for msgpackffi.lua. It, however, already has `encode_ext_cdata` and `ext_decoder` tables. So it seems we just need to add registration functions. Note: Don't sure whether encode_ext / decode_ext should operate directly on the Lua stack or we should separate those functions into get_from_stack+encode and decode+push_to_stack. (Vlad?) The functions that will be exposed by box should encode any box.error object into msgpack and decode any msgpacked box.error into box.error object. Build*Error() constructors should be called for this with appropriate arguments. So the decode function will contain the switch by an error types and call necessary Build*Error() functions. Maybe also will replace the error message, where needed. This way looks hacky, but at least it concentrates hacking in one place. When we'll remove C++ classes for errors, we can refactor this part. [1]: https://github.com/tarantool/tarantool/issues/4398#issuecomment-606937494 WBR, Alexander Turenko.