From: Alexander Turenko <alexander.turenko@tarantool.org> To: Roman Khabibov <roman.habibov@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [server-dev] Re: Re: [PATCH v2 1/2] lua: return getaddrinfo() errors Date: Fri, 6 Sep 2019 16:45:09 +0300 [thread overview] Message-ID: <20190906134509.egjpa2vxfdolkeme@tkn_work_nb> (raw) In-Reply-To: <868EAF2C-A491-46C9-AD37-7512D6CAB213@tarantool.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?
next prev parent reply other threads:[~2019-09-06 13:45 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-15 16:58 [tarantool-patches] [PATCH] " Roman Khabibov 2019-06-23 20:31 ` [tarantool-patches] " Alexander Turenko 2019-06-25 13:38 ` [tarantool-patches] Re: [PATCH v2 1/2] " Roman Khabibov 2019-07-09 8:04 ` Alexander Turenko 2019-07-10 2:16 ` Roman Khabibov 2019-07-23 12:39 ` Alexander Turenko 2019-07-26 13:48 ` Alexander Turenko 2019-08-05 13:36 ` Roman Khabibov 2019-08-29 0:45 ` Alexander Turenko [not found] ` <868EAF2C-A491-46C9-AD37-7512D6CAB213@tarantool.org> 2019-09-06 13:45 ` Alexander Turenko [this message] 2019-09-10 12:54 ` [tarantool-patches] Re: [server-dev] " Roman Khabibov 2019-11-01 14:39 ` [Tarantool-patches] [server-dev] Re: [tarantool-patches] " Alexander Turenko 2019-11-21 17:27 ` [Tarantool-patches] [tarantool-patches] [server-dev] " Roman Khabibov 2019-12-08 19:48 ` Alexander Turenko 2019-12-10 16:25 ` Roman Khabibov 2019-12-18 14:58 ` Alexander Turenko 2019-12-21 17:50 ` Roman Khabibov 2019-12-23 13:36 ` Alexander Turenko 2019-12-26 17:29 ` Roman Khabibov 2020-02-18 13:55 ` [Tarantool-patches] Fwd: " Roman Khabibov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190906134509.egjpa2vxfdolkeme@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=roman.habibov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [server-dev] Re: Re: [PATCH v2 1/2] lua: return getaddrinfo() errors' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox