From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9625E46970E for ; Mon, 23 Dec 2019 16:37:00 +0300 (MSK) Date: Mon, 23 Dec 2019 16:36:58 +0300 From: Alexander Turenko Message-ID: <20191223133658.odu5uysnpwlajg7a@tkn_work_nb> References: <20190829004547.k24fktai33tlbl73@tkn_work_nb> <868EAF2C-A491-46C9-AD37-7512D6CAB213@tarantool.org> <20190906134509.egjpa2vxfdolkeme@tkn_work_nb> <20191101143929.7gv53nqgscwpbayv@tkn_work_nb> <0D96FEEE-3814-4C90-980F-723A837F8157@tarantool.org> <20191208194800.b6oiyzy3zx23mv4h@tkn_work_nb> <35E941DB-8252-478B-80C2-592FFAA6011E@tarantool.org> <20191218145852.zvqd7kyuodyzv7cm@tkn_work_nb> <77F0F11F-1910-4078-B606-78336F63ACDB@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <77F0F11F-1910-4078-B606-78336F63ACDB@tarantool.org> Subject: Re: [Tarantool-patches] [tarantool-patches] [server-dev] Re: Re: [PATCH v2 1/2] lua: return getaddrinfo() errors List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.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.