Tarantool development patches archive
 help / color / mirror / Atom feed
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?

  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