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 B896620B17 for ; Fri, 6 Sep 2019 09:45:26 -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 wbCifZsm5fVd for ; Fri, 6 Sep 2019 09:45:26 -0400 (EDT) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 78FE320AE5 for ; Fri, 6 Sep 2019 09:45:26 -0400 (EDT) Date: Fri, 6 Sep 2019 16:45:09 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [server-dev] Re: Re: [PATCH v2 1/2] lua: return getaddrinfo() errors Message-ID: <20190906134509.egjpa2vxfdolkeme@tkn_work_nb> References: <20190615165829.11888-1-roman.habibov@tarantool.org> <20190623203141.snnjgyqrntr6okp4@tkn_work_nb> <8C0820C4-7F5E-40A9-A634-E449E700D816@tarantool.org> <20190709080407.sv5quzrxpbnijwp4@tkn_work_nb> <20190723123954.dgtfsetqlmjhued3@tkn_work_nb> <704142F3-802F-4C1C-8FA2-6155BFF9A951@tarantool.org> <20190829004547.k24fktai33tlbl73@tkn_work_nb> <868EAF2C-A491-46C9-AD37-7512D6CAB213@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <868EAF2C-A491-46C9-AD37-7512D6CAB213@tarantool.org> 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: Roman Khabibov Cc: tarantool-patches@freelists.org > +-- gh-4138 Check getaddrinfo() error from socket:connect() only. > +-- Error code and error message returned by getaddrinfo() depends > +-- on system's gai_strerror(). > > >> +test_run:cmd("setopt delimiter ';'") > >> +function check_err(err) > >> + if err == 'getaddrinfo: nodename nor servname provided, or not known' or > >> + err == 'getaddrinfo: Servname not supported for ai_socktype' or > >> + err == 'getaddrinfo: Name or service not known' then > >> + return true > >> + end > >> + return false > >> +end; > >> +test_run:cmd("setopt delimiter ''"); > >> + > >> +s, err = socket:connect('non_exists_hostname:3301') > >> +check_err(err) == true > >> + > >> test_run:cmd("clear filter”) > > > > It is better to verify it against ffi.C.gai_strerror(EAI_AGAIN) > > ffi.C.gai_strerror(EAI_NONAME) and don't rely on the fact that those > > messages don't changed across libc versions. The same for other tests. > Don’t know how to export this macroses/enums to Lua. More, it is too > complicated, because we need to covert cdata to Lua string. We discussed it voicely with Roman. The plan was to add needed constants to socket.c and expose into Lua as strings. We however don't expose GAI_* error codes, just messages from gai_strerror(). So maybe adding them just for tests isn't really worthful. > >> +-- gh-4138 Check getaddrinfo() error from socket:connect() only. > >> +-- Error code and error message returned by getaddrinfo() depends > >> +-- on system's gai_strerror(). So that there is no checking for > >> +-- certain error message. > >> +test_run:cmd("setopt delimiter ';'") > >> +--- > >> +- true > >> +... > >> +function check_err(err) > >> + if err == 'getaddrinfo: nodename nor servname provided, or not known' or > >> + err == 'getaddrinfo: Servname not supported for ai_socktype' or > >> + err == 'getaddrinfo: Name or service not known' then > >> + return true > >> + end > >> + return false > >> +end; > >> +--- > >> +... > >> +test_run:cmd("setopt delimiter ''"); > >> +--- > >> +- true > >> +... > >> +s, err = socket:connect('non_exists_hostname:3301') > > > > socket.connect() should be called, the colon here is the mistake. Please, look again to the code. One call is socket.connect(...), another is socket:connect(). Just in case, I suggest to add a patch to your patchset to keep youself from such mistakes: | diff --git a/src/lua/socket.lua b/src/lua/socket.lua | index e4815adbb..a1c13d4db 100644 | --- a/src/lua/socket.lua | +++ b/src/lua/socket.lua | @@ -1554,7 +1554,8 @@ local function lsocket_tcp() | end | | local function lsocket_connect(host, port) | - if host == nil or port == nil then | + if type(host) ~= 'string' or (type(port) ~= 'string' and | + type(port) ~= 'number') then | error("Usage: luasocket.connect(host, port)") | end | local s, err = tcp_connect(host, port) | @@ -1569,7 +1570,8 @@ local function lsocket_connect(host, port) | end | | local function lsocket_bind(host, port, backlog) | - if host == nil or port == nil then | + if type(host) ~= 'string' or (type(port) ~= 'string' and | + type(port) ~= 'number') then | error("Usage: luasocket.bind(host, port [, backlog])") | end | local function prepare(s) return backlog end > > I think we should test both native and Lua Socket API, because both were > > changed. > > > > Should we ever change Lua Socket API? Please, verify it with a > > documentation and an actual behaviour of the external module. We emulate > > it, so we should be compatible. Are you look into this?