> On 3 May 2018, at 19:54, Sadasiva Gujjarlapudi <[email protected]> > wrote: > > `check_args` checked for the "exact number of" arguments available on the Lua > stack as > given in the second param(nb).
You’re right ! Good catch. Can you provide the patch with this lot of explanation for the commit ? Like this: http://git.haproxy.org/?p=haproxy-1.8.git;a=commitdiff;h=05657bd24ebaf20e5c508a435be9a0830591f033 Thanks, Thierry > So `close` was expecting exactly "one" argument on the Lua stack. > But when `close` was called from xxx_write_yield(), Lua stack had 3 arguments. > > So close threw the exception with message "'close' needs 1 arguments". > > "close_helper" function removed the Lua stack argument count check and only > checked > if the first argument was a socket. > > ---- > void check_args(lua_State *L, int nb, char *fcn) > { > if (lua_gettop(L) == nb) > return; > WILL_LJMP(luaL_error(L, "'%s' needs %d arguments", fcn, nb)); > } > ---- > > Please let me know if you need more information. > Thanks, > Sada. > > > On Thu, May 3, 2018 at 12:16 AM, Thierry Fournier <[email protected]> > wrote: > This function checks that one argument are present in the stack, at the > bottom of the stack (position 1). > > MAY_LJMP(check_args(L, 1, "close")); > > The stack is kept between calls, so the position 1 will contains anything. > The content of the position 1 in the stack is a struct associated with the > socket object. > > The function close is called byt the function xxx_write() which is a callback > of the Lua function “send()”. This send function have the same same first > argument then the Lua function "close()”, so the stack contains a socket > object > at position 1. > > I’m sure because the following line, just after the stack check gets > this argument: > > socket = MAY_LJMP(hlua_checksocket(L, 1)); > > If your patch works, this is a proof that the stack contains expected value > at position 1, so removing the stack check doesn’t fix anything. > > Maybe I miss something, or you encounters a bug caused by anything else. > Do you have a method for reproducing a bug ? > > BR, > Thierry > > > > > On 3 May 2018, at 08:56, Thierry Fournier <[email protected]> wrote: > > > > Hi, > > > > I’m not sure about your patch because the values in the stack are kept. > > I need to be focus to read it, and unfortunately I’m very busy. > > I try to check this morning, during the french strikes... > > > > Thierry > > > > > >> On 25 Apr 2018, at 20:26, Sadasiva Gujjarlapudi <[email protected]> > >> wrote: > >> > >> cc: Haproxy > >> > >> `hlua_socket_close` expected exactly one argument on stack. > >> But when it was called from `hlua_socket_write_yield`, it found more than > >> one argument on stack and threw an error. > >> > >> This patch introduced new helper function `hlua_socket_close_helper`, > >> which doesn't check arguments count. > >> Please let me know if you need more information. > >> > >> Thanks, > >> Sada. > >> > >> > >> On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier > >> <[email protected]> wrote: > >> Hi, > >> > >> do you have a little bit of context. It is not easy to read a patch without > >> the associated explanation. > >> > >> Thanks, > >> Thierry > >> > >> > >> > >>> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote: > >>> > >>> --- > >>> src/hlua.c | 14 ++++++++++---- > >>> 1 file changed, 10 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/src/hlua.c b/src/hlua.c > >>> index 60cf8f94..0585a1e7 100644 > >>> --- a/src/hlua.c > >>> +++ b/src/hlua.c > >>> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L) > >>> /* The close function send shutdown signal and break the > >>> * links between the stream and the object. > >>> */ > >>> -__LJMP static int hlua_socket_close(lua_State *L) > >>> +__LJMP static int hlua_socket_close_helper(lua_State *L) > >>> { > >>> struct hlua_socket *socket; > >>> struct appctx *appctx; > >>> struct xref *peer; > >>> > >>> - MAY_LJMP(check_args(L, 1, "close")); > >>> - > >>> socket = MAY_LJMP(hlua_checksocket(L, 1)); > >>> > >>> /* Check if we run on the same thread than the xreator thread. > >>> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L) > >>> return 0; > >>> } > >>> > >>> +/* The close function calls close_helper. > >>> + */ > >>> +__LJMP static int hlua_socket_close(lua_State *L) > >>> +{ > >>> + MAY_LJMP(check_args(L, 1, "close")); > >>> + return hlua_socket_close_helper(L); > >>> +} > >>> + > >>> /* This Lua function assumes that the stack contain three parameters. > >>> * 1 - USERDATA containing a struct socket > >>> * 2 - INTEGER with values of the macro defined below > >>> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State > >>> *L,int status, lua_KContext > >>> if (len == -1) > >>> s->req.flags |= CF_WAKE_WRITE; > >>> > >>> - MAY_LJMP(hlua_socket_close(L)); > >>> + MAY_LJMP(hlua_socket_close_helper(L)); > >>> lua_pop(L, 1); > >>> lua_pushinteger(L, -1); > >>> xref_unlock(&socket->xref, peer); > >>> -- > >>> 2.17.0 > >>> > >> > >> > > > >
