From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 216252AD30 for ; Thu, 28 Mar 2019 14:40:04 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dQ7QJzGaGHMM for ; Thu, 28 Mar 2019 14:40:04 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 55B982AA31 for ; Thu, 28 Mar 2019 14:40:03 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v9 4/7] lua: remove exceptions from function luaL_tofield() References: <20190328175410.GC4864@tarantool.org> From: Vladislav Shpilevoy Message-ID: <58e2a19a-e6ce-bfaf-d926-c216ae83f197@tarantool.org> Date: Thu, 28 Mar 2019 21:40:00 +0300 MIME-Version: 1.0 In-Reply-To: <20190328175410.GC4864@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Mergen Imeev Cc: tarantool-patches@freelists.org Thanks for the fixes! I've pushed my review fixes on the branch, and dropped at the end of the email. But I've not tested them, so I propose you to do that, if you agree with the concept. ========================================================= diff --git a/src/lua/utils.c b/src/lua/utils.c index 6150e84ff..38245a979 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -399,47 +399,58 @@ lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg, } /** - * Check that the field LUAL_SERIALIZE of the metatable is - * available. If so, create field using it. - * - * This function returns two values on stack. On top of the stack - * is boolean that shows if error happened. On the next position - * can be boolean or result of execution of function from the - * metatable. If it is boolean, that if its value is false than - * there were no metatables.It its value is true than there was - * string in that field of metatable. - * - * @param L Lua State. + * A helper structure to simplify safe call of __serialize method. + * It passes some arguments into the serializer called via pcall, + * and carries out some results. + */ +struct lua_serialize_status { + /** + * True if an attempt to call __serialize has failed. A + * diag message is set. + */ + bool is_error; + /** + * True, if __serialize exists. Otherwise an ordinary + * default serialization is used. + */ + bool is_serialize_used; + /** Parameters, passed originally to luaL_tofield. */ + struct luaL_serializer *cfg; + struct luaL_field *field; +}; + +/** + * Call __serialize method of a table object if the former exists. + * The function expects 2 values pushed onto the Lua stack: a + * value to serialize, and a pointer at a struct + * lua_serialize_status object. If __serialize exists and is + * correct, then one value is pushed as a result of serialization. + * Otherwise is_error and is_serialize_used attributes of status + * should be checked. */ static int -get_metafield_serialize(struct lua_State *L) +lua_field_try_serialize(struct lua_State *L) { - if (luaL_getmetafield(L, 1, LUAL_SERIALIZE) == 0) { - lua_pushboolean(L, false); - lua_pushboolean(L, false); - return 2; - } - bool is_error = false; - struct luaL_serializer *cfg = - (struct luaL_serializer *)lua_touserdata(L, 2); - struct luaL_field *field = - (struct luaL_field *)lua_touserdata(L, 3); - + struct lua_serialize_status *s = + (struct lua_serialize_status *) lua_touserdata(L, 2); + s->is_serialize_used = (luaL_getmetafield(L, 1, LUAL_SERIALIZE) != 0); + s->is_error = false; + if (! s->is_serialize_used) + return 0; + struct luaL_serializer *cfg = s->cfg; + struct luaL_field *field = s->field; if (lua_isfunction(L, -1)) { /* copy object itself */ lua_pushvalue(L, 1); lua_call(L, 1, 1); - is_error = (luaL_tofield(L, cfg, -1, field) != 0); - lua_pushboolean(L, is_error); - return 2; + s->is_error = (luaL_tofield(L, cfg, -1, field) != 0); + return s->is_error ? 0 : 1; } - if (!lua_isstring(L, -1)) { diag_set(LuajitError, "invalid " LUAL_SERIALIZE " value"); - lua_pushboolean(L, true); - return 2; + s->is_error = true; + return 0; } - const char *type = lua_tostring(L, -1); if (strcmp(type, "array") == 0 || strcmp(type, "seq") == 0 || strcmp(type, "sequence") == 0) { @@ -449,9 +460,7 @@ get_metafield_serialize(struct lua_State *L) if (cfg->has_compact && type[3] == '\0') field->compact = true; lua_pop(L, 1); /* type */ - lua_pushboolean(L, true); - lua_pushboolean(L, false); - return 2; + return 1; } else if (strcmp(type, "map") == 0 || strcmp(type, "mapping") == 0) { field->type = MP_MAP; /* Override type */ field->size = luaL_maplen(L, 1); @@ -459,13 +468,11 @@ get_metafield_serialize(struct lua_State *L) if (cfg->has_compact && type[3] == '\0') field->compact = true; lua_pop(L, 1); /* type */ - lua_pushboolean(L, true); - lua_pushboolean(L, false); - return 2; + return 1; } else { diag_set(LuajitError, "invalid " LUAL_SERIALIZE " value"); - lua_pushboolean(L, true); - return 2; + s->is_error = true; + return 0; } } @@ -478,27 +485,19 @@ lua_field_inspect_table(struct lua_State *L, struct luaL_serializer *cfg, uint32_t max = 0; if (cfg->encode_load_metatables) { - lua_pushcfunction(L, get_metafield_serialize); + struct lua_serialize_status s; + s.cfg = cfg; + s.field = field; + lua_pushcfunction(L, lua_field_try_serialize); lua_pushvalue(L, idx); - lua_pushlightuserdata(L, cfg); - lua_pushlightuserdata(L, field); - if (lua_pcall(L, 3, 2, 0) != 0) { + lua_pushlightuserdata(L, &s); + if (lua_pcall(L, 2, 1, 0) != 0) { diag_set(LuajitError, lua_tostring(L, -1)); return -1; } - bool is_error = lua_toboolean(L, -1); - bool is_bool = lua_isboolean(L, -2); - if (is_error) { - lua_pop(L, 2); + if (s.is_error) return -1; - } - if (is_bool) { - bool is_metatable_available = lua_toboolean(L, -2); - lua_pop(L, 2); - if (is_metatable_available) - return 0; - } else { - lua_pop(L, 1); + if (s.is_serialize_used) { lua_replace(L, idx); return 0; }