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 9543146970E for ; Wed, 22 Jan 2020 05:25:31 +0300 (MSK) Date: Wed, 22 Jan 2020 05:25:38 +0300 From: Alexander Turenko Message-ID: <20200122022537.tygaowsn5remjee4@tkn_work_nb> References: <1065f691482e681b779abd3698ec4489267c11fd.1579558507.git.v.shpilevoy@tarantool.org> <20200121002441.sysri34odzjrpjzu@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 1/1] box: export box_session_push to the public C API List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org > > I see that box_return_tuple() receives (box_function_ctx_t *) as the > > first argument and wonder why box_session_push() does not. If we able to > > determine where to and how to send a push w/o the context, then the > > context is not necessary in box_return_tuple() too? I don't very > > familiar with this part of codebase, so it is just question for now. > > I'll look around soon if time permits. > > Context of session push is a session. It is available in fiber, which > is a global variable. > > Context of a function is its port - a storage for returned values. Port > is not available in the fiber, and therefore is not stored anywhere > globally. It needs to be passed explicitly. We pass it as an opaque > pointer box_function_ctx_t*. > > We can add a context to the push, but then we need to create a public > version of struct session. Something like 'box_session_t', which would > be an alias for 'struct session'. And have a function like > 'box_session_current()', which in turn won't have a context. > > I think we need to introduce box_session_t only if we are going to > allow users to access foreign sessions. Otherwise it is always the > user's own session and is available in the fiber anyway. But box_return_tuple() can create a port on demand, just like you do in box_session_push(), right? I don't see any difference between those functions in context of the question. If box_return_tuple() could be implemented with or without explicit (box_function_ctx_t *) argument in the past, then someone made the decision to implement it with the argument and there is (hopefully) some reasoning under this decision. If so, it may be applicable to box_session_push(). Or there is a reason why it is not applicable here. > + /* > + * Create a new thread and pop it immediately from the > + * main stack. So as it would not stay there in case > + * something would throw in this function. > + */ > + int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); > + /* > + * MessagePack -> Lua -> YAML. The middle is not really > + * needed here, but there is no MessagePack -> YAML > + * encoder yet. > + */ > + const char *data = port->data; > + luamp_decode(L, luaL_msgpack_default, &data); > + int rc = lua_yaml_encode(L, luaL_yaml_default, "!push!", > + "tag:tarantool.io/push,2018"); > ================================================================================ > > luamp_decode and lua_yaml_encode can throw a Lua error. Don't know how to > fix that except introduction of a MessagePack -> YAML converter, which seems > a pretty big and independent task. > > ================================================================================ It seems that it is usually handled with lua_pushcfunction() / lua_pushcclosure() + lua_pcall().