From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (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 114A1446429 for ; Sun, 29 Mar 2020 12:12:47 +0300 (MSK) Date: Sun, 29 Mar 2020 12:12:51 +0300 From: Alexander Turenko Message-ID: <20200329091251.eihtejl6hrec4pon@tkn_work_nb> References: <20200312102434.97300-1-roman.habibov@tarantool.org> <20200312102434.97300-2-roman.habibov@tarantool.org> <20200329085141.GC328@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200329085141.GC328@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 1/2] coio/say: fix getaddrinfo error handling on macOS List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org > > diff --git a/src/lib/core/say.c b/src/lib/core/say.c > > index 64a637c58..8ad88ad57 100644 > > --- a/src/lib/core/say.c > > +++ b/src/lib/core/say.c > > @@ -459,14 +459,17 @@ static inline int > > syslog_connect_unix(const char *path) > > { > > int fd = socket(PF_UNIX, SOCK_DGRAM, 0); > > - if (fd < 0) > > + if (fd < 0) { > > + diag_set(SystemError, "socket"); > This error message gives nothing. Please, describe the error behind it > using the strerror(errno) | void | SystemError::log() const | { | say_file_line(S_SYSERROR, file, line, strerror(saved_errno), | "SystemError %s", errmsg); | } https://github.com/tarantool/tarantool/blob/eaa860885dc5708956094845e20f5bc81275ca26/src/lib/core/exception.cc#L153-L158 > > diff --git a/test/unit/coio.cc b/test/unit/coio.cc > > index bb8bd7131..957c58ede 100644 > > --- a/test/unit/coio.cc > > +++ b/test/unit/coio.cc > > @@ -72,7 +72,7 @@ static void > > test_getaddrinfo(void) > > { > > header(); > > - plan(1); > > + plan(3); > > const char *host = "127.0.0.1"; > > const char *port = "3333"; > > struct addrinfo *i; > > @@ -81,6 +81,33 @@ test_getaddrinfo(void) > > is(rc, 0, "getaddrinfo"); > > freeaddrinfo(i); > > > > + /* > > + * gh-4138: Check getaddrinfo() retval and diagnostics > > + * area. > > + */ > > + rc = coio_getaddrinfo("non_exists_hostname", port, NULL, &i, > > + 15768000000); > > + isnt(rc, 0, "getaddrinfo retval"); > > + const char *errmsg = diag_get()->last->errmsg; > > + const char *exp_errmsg_1 = "getaddrinfo: nodename nor servname provided" > > + ", or not known"; > > + const char *exp_errmsg_2 = "getaddrinfo: Servname not supported for " > > + "ai_socktype"; > > + const char *exp_errmsg_3 = "getaddrinfo: Name or service not known"; > > + const char *exp_errmsg_4 = "getaddrinfo: hostname nor servname provided" > > + ", or not known"; > > + const char *exp_errmsg_5 = "getaddrinfo: Temporary failure in name " > > + "resolution"; > > + const char *exp_errmsg_6 = "getaddrinfo: Name could not be resolved at " > > + "this time"; > > + bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 || > > + strcmp(errmsg, exp_errmsg_2) == 0 || > > + strcmp(errmsg, exp_errmsg_3) == 0 || > > + strcmp(errmsg, exp_errmsg_4) == 0 || > > + strcmp(errmsg, exp_errmsg_5) == 0 || > > + strcmp(errmsg, exp_errmsg_6) == 0; > > + is(is_match_with_exp, true, "getaddrinfo error message"); > > + > Why did you made such a test - you're not sure which one will be > triggered? Can you create a test that will check all possible errors? I asked to check for certain errors rather than that some SystemError with '^getaddrinfo: .*$' message raised. Sure, the return value of getaddrinfo() depends on network conditions (I guess it depends of a DNS server: whether it reports temporary or persistent error; but also whether a network interface is up at all). gai_strerror() output depends of OS. I asked to comment messages with EAI_* constants that corresponds to them, but it seems it was missed. Sure, proper description is necessary here. Nope, we unable to control network conditions and OS here and cannot create a test case for each of those situations.