From: Alexander Turenko <alexander.turenko@tarantool.org> To: Roman Khabibov <roman.habibov@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [tarantool-patches] [server-dev] Re: Re: [PATCH v2 1/2] lua: return getaddrinfo() errors Date: Mon, 23 Dec 2019 16:36:58 +0300 [thread overview] Message-ID: <20191223133658.odu5uysnpwlajg7a@tkn_work_nb> (raw) In-Reply-To: <77F0F11F-1910-4078-B606-78336F63ACDB@tarantool.org> Comments below are trivial. Let's fix and proceed with the next reviewer (Sergey O.). Please, resend the patchset for him. I still think you should run CI on all targets (push to ...-full-ci branch). LGTM except minor comments below. WBR, Alexander Turenko. > @@ -1360,8 +1363,12 @@ local function lsocket_tcp_connect(self, host, port) > -- This function is broken by design > local ga_opts = { family = 'AF_INET', type = 'SOCK_STREAM' } > local timeout = deadline - fiber.clock() > - local dns = getaddrinfo(host, port, timeout, ga_opts) > - if dns == nil or #dns == 0 then > + local dns, err = getaddrinfo(host, port, timeout, ga_opts) > + if dns == nil then > + self._errno = boxerrno() Master's socket _errno was set to EINVAL before this patch. Let's keep this behaviour, but change the 2nd return value. I mean: | if dns == nil then | self._errno = boxerrno.EINVAL | return nil, err | end | if #dns == 0 then | self._errno = boxerrno.EINVAL | return nil, socket_error(self) | end > + return nil, err > + end > + if #dns == 0 then > self._errno = boxerrno.EINVAL > return nil, socket_error(self) > end > diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua > index c72d41763..7086e496b 100644 > --- a/test/app/socket.test.lua > +++ b/test/app/socket.test.lua > @@ -960,6 +968,27 @@ server:close() > > test_run:cmd("clear filter") > > +-- 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; You added the following error message to coio_getaddrinfo() unit test in the first commit: | const char *exp_errmsg_4 = "getaddrinfo: hostname nor servname provided" | ", or not known"; So it worth to add it here too. We'll enable the test of FreeBSD sooner or later. Also I got EAI_AGAIN on my system now (maybe something was changed on DNS server side). Let's add it too (for both commits). > diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua > index 8e65ff470..9c9aee1ad 100644 > --- a/test/box/net.box.test.lua > +++ b/test/box/net.box.test.lua > @@ -1582,4 +1582,21 @@ test_run:wait_log('default', '00000030:.*', nil, 10) > -- we expect nothing below, so don't wait > test_run:grep_log('default', '00000040:.*') > > +-- gh-4138 Check getaddrinfo() error from 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 ''"); Same here.
next prev parent reply other threads:[~2019-12-23 13:37 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 ` [tarantool-patches] Re: [server-dev] " Alexander Turenko 2019-09-10 12:54 ` 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 [this message] 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=20191223133658.odu5uysnpwlajg7a@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=roman.habibov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [tarantool-patches] [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