* [tarantool-patches] [PATCH v2 0/2] take getaddrinfo() errors into account
@ 2019-06-25 13:38 Roman Khabibov
2019-06-25 13:38 ` [tarantool-patches] [PATCH v2 2/2] say: " Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-06-25 13:38 UTC (permalink / raw)
To: tarantool-patches; +Cc: alexander.turenko
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on Mac OS, because they are greater than 0 on
Mac OS (checking for errors was rc < 0).
Roman Khabibov (2):
lua: return getaddrinfo() errors
say: take getaddrinfo() errors into account
src/box/lua/net_box.lua | 7 +++++--
src/lib/core/coio_task.c | 2 +-
src/lib/core/say.c | 3 ++-
src/lua/socket.c | 17 ++++++++++++++++-
src/lua/socket.lua | 37 +++++++++++++++++++++++--------------
test/app/socket.result | 21 ++++++++++++++++++++-
test/app/socket.test.lua | 10 ++++++++++
test/box-tap/cfg.test.lua | 19 ++++++++++++++++++-
test/box/net.box.result | 11 +++++++++++
test/box/net.box.test.lua | 8 ++++++++
test/unit/coio.cc | 7 ++++++-
test/unit/coio.result | 4 +++-
12 files changed, 123 insertions(+), 23 deletions(-)
--
Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4138-getaddrinfo
Issue: https://github.com/tarantool/tarantool/issues/4138
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-06-25 13:38 [tarantool-patches] [PATCH v2 0/2] take getaddrinfo() errors into account Roman Khabibov
@ 2019-06-25 13:38 ` Roman Khabibov
2019-07-23 14:52 ` [tarantool-patches] " Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-06-25 13:38 UTC (permalink / raw)
To: tarantool-patches; +Cc: alexander.turenko
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on Mac OS, because they are greater than 0 on
Mac OS (checking for errors was rc < 0).
Part of #4138
---
src/lib/core/say.c | 3 ++-
test/box-tap/cfg.test.lua | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 0b2cf2c34..c927f9fc0 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -506,7 +506,7 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
@@ -594,6 +594,7 @@ log_syslog_init(struct log *log, const char *init_str)
log->fd = log_syslog_connect(log);
if (log->fd < 0) {
/* syslog indent is freed in atexit(). */
+ diag_log();
diag_set(SystemError, "syslog logger: %s", strerror(errno));
return -1;
}
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index 55de5e41c..d92e9140d 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
local fio = require('fio')
local uuid = require('uuid')
local msgpack = require('msgpack')
-test:plan(104)
+test:plan(105)
--------------------------------------------------------------------------------
-- Invalid values
@@ -566,6 +566,23 @@ os.exit(0)
]]
test:is(run_script(code), 0, "log_nonblock")
+--
+-- gh-4138: check getaddrinfo() error and panic after that.
+--
+code=[[
+local socket = require('socket')
+local log = require('log')
+local fio = require('fio')
+
+path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock')
+unix_socket = socket('AF_UNIX', 'SOCK_DGRAM', 0)
+unix_socket:bind('unix/', path)
+
+opt = string.format("syslog:server=non_exists_hostname:%s,identity=tarantool", path)
+box.cfg{log = opt, log_nonblock=true}
+]]
+test:is(run_script(code), PANIC, "log_nonblock")
+
--
-- Crash (instead of panic) when trying to recover a huge tuple.
--
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-06-25 13:38 ` [tarantool-patches] [PATCH v2 2/2] say: " Roman Khabibov
@ 2019-07-23 14:52 ` Alexander Turenko
2019-08-05 13:32 ` Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-07-23 14:52 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
> @@ -594,6 +594,7 @@ log_syslog_init(struct log *log, const char *init_str)
> log->fd = log_syslog_connect(log);
> if (log->fd < 0) {
> /* syslog indent is freed in atexit(). */
> + diag_log();
1. It is need to be properly commented: we need to log a diagnostics
here until stacked diagnostics will be implemented (#1148).
2. syslog_connect_unix() does not set a diag, diag_log() will lead to an
assertion fail.
3. I would mention this change in the commit message, because it is not
part of the problem with Mac OS you described in it.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-07-23 14:52 ` [tarantool-patches] " Alexander Turenko
@ 2019-08-05 13:32 ` Roman Khabibov
2019-08-28 21:34 ` Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-08-05 13:32 UTC (permalink / raw)
To: tarantool-patches; +Cc: Alexander Turenko
> On Jul 23, 2019, at 17:52, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
>
>> @@ -594,6 +594,7 @@ log_syslog_init(struct log *log, const char *init_str)
>> log->fd = log_syslog_connect(log);
>> if (log->fd < 0) {
>> /* syslog indent is freed in atexit(). */
>> + diag_log();
>
> 1. It is need to be properly commented: we need to log a diagnostics
> here until stacked diagnostics will be implemented (#1148).
>
> 2. syslog_connect_unix() does not set a diag, diag_log() will lead to an
> assertion fail.
>
> 3. I would mention this change in the commit message, because it is not
> part of the problem with Mac OS you described in it.
@@ -506,10 +506,15 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
goto out;
}
commit f5b19e933fbf2eb3784a0c3cc28d999a3fa85abe
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 30 15:39:21 2019 +0300
coio/say: take getaddrinfo() errors into account
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on Mac OS, because they are greater than 0 on
Mac OS (assumption "rc < 0" in commit ea1da04 is incorrect for
Mac OS).
* diag log() in say.c was added, because we need to log a
diagnostics here until stacked diagnostics will be implemented
(#1148).
Need for #4138
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index 908b336ed..83f669d05 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
return -1; /* timed out or cancelled */
/* Task finished */
- if (task->rc < 0) {
+ if (task->rc != 0) {
/* getaddrinfo() failed */
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 0b2cf2c34..a45595443 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -506,10 +506,15 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
goto out;
}
for (ptr = inf; ptr; ptr = ptr->ai_next) {
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index 55de5e41c..d92e9140d 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
local fio = require('fio')
local uuid = require('uuid')
local msgpack = require('msgpack')
-test:plan(104)
+test:plan(105)
--------------------------------------------------------------------------------
-- Invalid values
@@ -566,6 +566,23 @@ os.exit(0)
]]
test:is(run_script(code), 0, "log_nonblock")
+--
+-- gh-4138: check getaddrinfo() error and panic after that.
+--
+code=[[
+local socket = require('socket')
+local log = require('log')
+local fio = require('fio')
+
+path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock')
+unix_socket = socket('AF_UNIX', 'SOCK_DGRAM', 0)
+unix_socket:bind('unix/', path)
+
+opt = string.format("syslog:server=non_exists_hostname:%s,identity=tarantool", path)
+box.cfg{log = opt, log_nonblock=true}
+]]
+test:is(run_script(code), PANIC, "log_nonblock")
+
--
-- Crash (instead of panic) when trying to recover a huge tuple.
--
diff --git a/test/unit/coio.cc b/test/unit/coio.cc
index bb8bd7131..a70d3254d 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,12 @@ test_getaddrinfo(void)
is(rc, 0, "getaddrinfo");
freeaddrinfo(i);
+ /* gh-4138: Check getaddrinfo() error. */
+ isnt(coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1), 0,
+ "getaddrinfo error");
+ isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
+ "getaddrinfo error message");
+
/*
* gh-4209: 0 timeout should not be a special value and
* detach a task. Before a fix it led to segfault
diff --git a/test/unit/coio.result b/test/unit/coio.result
index 5019fa48a..49759b747 100644
--- a/test/unit/coio.result
+++ b/test/unit/coio.result
@@ -7,6 +7,8 @@
# call done with res 0
*** test_call_f: done ***
*** test_getaddrinfo ***
-1..1
+1..3
ok 1 - getaddrinfo
+ok 2 - getaddrinfo error
+ok 3 - getaddrinfo error message
*** test_getaddrinfo: done ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-08-05 13:32 ` Roman Khabibov
@ 2019-08-28 21:34 ` Alexander Turenko
2019-08-29 0:51 ` Alexander Turenko
[not found] ` <8E98F721-601F-436D-8F0A-5E399D8F7CAB@tarantool.org>
0 siblings, 2 replies; 19+ messages in thread
From: Alexander Turenko @ 2019-08-28 21:34 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
This particular patch is mostly okay, but I would work a bit more on
tests and minor details.
Please, consider comments below.
WBR, Alexander Turenko.
On Mon, Aug 05, 2019 at 04:32:41PM +0300, Roman Khabibov wrote:
>
>
> > On Jul 23, 2019, at 17:52, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
> >
> >> @@ -594,6 +594,7 @@ log_syslog_init(struct log *log, const char *init_str)
> >> log->fd = log_syslog_connect(log);
> >> if (log->fd < 0) {
> >> /* syslog indent is freed in atexit(). */
> >> + diag_log();
> >
> > 1. It is need to be properly commented: we need to log a diagnostics
> > here until stacked diagnostics will be implemented (#1148).
> >
> > 2. syslog_connect_unix() does not set a diag, diag_log() will lead to an
> > assertion fail.
> >
> > 3. I would mention this change in the commit message, because it is not
> > part of the problem with Mac OS you described in it.
> @@ -506,10 +506,15 @@ syslog_connect_remote(const char *server_address)
> hints.ai_protocol = IPPROTO_UDP;
>
> ret = getaddrinfo(remote, portnum, &hints, &inf);
> - if (ret < 0) {
> + if (ret != 0) {
> errno = EIO;
> diag_set(SystemError, "getaddrinfo: %s",
> gai_strerror(ret));
> + /*
> + * We need to log a diagnostics here until stacked
> + * diagnostics will be implemented (#1148).
> + */
> + diag_log();
It is not the only error that is possible in this function, but others
will not be logged. I think the logging should be added before replacing
the diagnostic with the next one:
| --- a/src/lib/core/say.c
| +++ b/src/lib/core/say.c
| @@ -593,6 +593,7 @@ log_syslog_init(struct log *log, const char *init_str)
| say_free_syslog_opts(&opts);
| log->fd = log_syslog_connect(log);
| if (log->fd < 0) {
| + /* XXX: comment. */
| + diag_log();
| /* syslog indent is freed in atexit(). */
| diag_set(SystemError, "syslog logger: %s", strerror(errno));
| return -1;
> commit f5b19e933fbf2eb3784a0c3cc28d999a3fa85abe
> Author: Roman Khabibov <roman.habibov@tarantool.org>
> Date: Tue Jul 30 15:39:21 2019 +0300
>
> coio/say: take getaddrinfo() errors into account
I would say that this fix is for Mac OS in the commit header if
possible. Say, 'coio/say: fix getaddrinfo error handling on Mac OS'.
>
> Before this patch, branch when getaddrinfo() returns error codes
> couldn't be reached on Mac OS, because they are greater than 0 on
> Mac OS (assumption "rc < 0" in commit ea1da04 is incorrect for
> Mac OS).
>
> * diag log() in say.c was added, because we need to log a
> diagnostics here until stacked diagnostics will be implemented
> (#1148).
I would say, because otherwise it will be hid by the following
diagnostic and then say that it should be handler in a better way after
#1148.
Asterisk here is redundant, because it is not a list.
>
> Need for #4138
>
> diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
> index 908b336ed..83f669d05 100644
> --- a/src/lib/core/coio_task.c
> +++ b/src/lib/core/coio_task.c
> @@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
> return -1; /* timed out or cancelled */
>
> /* Task finished */
> - if (task->rc < 0) {
> + if (task->rc != 0) {
> /* getaddrinfo() failed */
> errno = EIO;
> diag_set(SystemError, "getaddrinfo: %s",
> diff --git a/src/lib/core/say.c b/src/lib/core/say.c
> index 0b2cf2c34..a45595443 100644
> --- a/src/lib/core/say.c
> +++ b/src/lib/core/say.c
> @@ -506,10 +506,15 @@ syslog_connect_remote(const char *server_address)
> hints.ai_protocol = IPPROTO_UDP;
>
> ret = getaddrinfo(remote, portnum, &hints, &inf);
> - if (ret < 0) {
> + if (ret != 0) {
> errno = EIO;
> diag_set(SystemError, "getaddrinfo: %s",
> gai_strerror(ret));
> + /*
> + * We need to log a diagnostics here until stacked
> + * diagnostics will be implemented (#1148).
> + */
> + diag_log();
> goto out;
> }
> for (ptr = inf; ptr; ptr = ptr->ai_next) {
> diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
> index 55de5e41c..d92e9140d 100755
> --- a/test/box-tap/cfg.test.lua
> +++ b/test/box-tap/cfg.test.lua
> @@ -6,7 +6,7 @@ local socket = require('socket')
> local fio = require('fio')
> local uuid = require('uuid')
> local msgpack = require('msgpack')
> -test:plan(104)
> +test:plan(105)
>
> --------------------------------------------------------------------------------
> -- Invalid values
> @@ -566,6 +566,23 @@ os.exit(0)
> ]]
> test:is(run_script(code), 0, "log_nonblock")
>
> +--
> +-- gh-4138: check getaddrinfo() error and panic after that.
> +--
> +code=[[
> +local socket = require('socket')
> +local log = require('log')
> +local fio = require('fio')
> +
> +path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock')
> +unix_socket = socket('AF_UNIX', 'SOCK_DGRAM', 0)
> +unix_socket:bind('unix/', path)
> +
> +opt = string.format("syslog:server=non_exists_hostname:%s,identity=tarantool", path)
> +box.cfg{log = opt, log_nonblock=true}
log_nonblock is not needed here, so it is better to remove it.
box.cfg{log = 'syslog:server=non_exists_hostname:3301'} is enough, not need to
form a file path, no need identity, no need requiring socket, log and fio.
The test passes even before the patch, so what it is intended to test? I
think we should write a test that verifies stderr output to find all log
messages we expect to appear in the case:
Linux:
| SystemError getaddrinfo: Temporary failure in name resolution: Input/output error
| SystemError syslog logger: Input/output error: Input/output error
| failed to initialize logging subsystem
gai_strerror() message corresponds to EAI_AGAIN.
Mac OS:
| SystemError getaddrinfo: nodename nor servname provided, or not known: Input/output error
| SystemError syslog logger: Input/output error: Input/output error
| failed to initialize logging subsystem
gai_strerror() message corresponds to EAI_NONAME.
I propose to call ffi.C.gai_strerror() right from a test to form two
error messages and verify that the actual input match one of them.
If it is hard to catch stderr, then let's proceed w/o this test. However
I think it is doable.
I also propose to test error messages in the similar way (using
ffi.C.gai_strerror(GAI_AGAIN) and ffi.C.gai_strerror(GAI_NONAME)) in
test cases in second patch of the patchset.
> +]]
> +test:is(run_script(code), PANIC, "log_nonblock")
> +
> --
> -- Crash (instead of panic) when trying to recover a huge tuple.
> --
> diff --git a/test/unit/coio.cc b/test/unit/coio.cc
> index bb8bd7131..a70d3254d 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,12 @@ test_getaddrinfo(void)
> is(rc, 0, "getaddrinfo");
> freeaddrinfo(i);
>
> + /* gh-4138: Check getaddrinfo() error. */
> + isnt(coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1), 0,
> + "getaddrinfo error");
I would say 'getaddrinfo retval' instead 'getaddrinfo error'.
Use `rc = coio_getaddrinfo(<...>)` as above within this function, it
reads easier.
> + isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
> + "getaddrinfo error message");
> +
I propose to verify the entire error message using
gai_strerror(GAI_AGAIN) and gai_strerror(GAI_NONAME)—just as proposed
above for a log message.
> /*
> * gh-4209: 0 timeout should not be a special value and
> * detach a task. Before a fix it led to segfault
> diff --git a/test/unit/coio.result b/test/unit/coio.result
> index 5019fa48a..49759b747 100644
> --- a/test/unit/coio.result
> +++ b/test/unit/coio.result
> @@ -7,6 +7,8 @@
> # call done with res 0
> *** test_call_f: done ***
> *** test_getaddrinfo ***
> -1..1
> +1..3
> ok 1 - getaddrinfo
> +ok 2 - getaddrinfo error
> +ok 3 - getaddrinfo error message
> *** test_getaddrinfo: done ***
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-08-28 21:34 ` Alexander Turenko
@ 2019-08-29 0:51 ` Alexander Turenko
[not found] ` <8E98F721-601F-436D-8F0A-5E399D8F7CAB@tarantool.org>
1 sibling, 0 replies; 19+ messages in thread
From: Alexander Turenko @ 2019-08-29 0:51 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
> Linux:
>
> | SystemError getaddrinfo: Temporary failure in name resolution: Input/output error
> | SystemError syslog logger: Input/output error: Input/output error
> | failed to initialize logging subsystem
>
> gai_strerror() message corresponds to EAI_AGAIN.
>
> Mac OS:
>
> | SystemError getaddrinfo: nodename nor servname provided, or not known: Input/output error
> | SystemError syslog logger: Input/output error: Input/output error
> | failed to initialize logging subsystem
>
> gai_strerror() message corresponds to EAI_NONAME.
I see now that Linux also can give EAI_NONAME in the case. It seems it
depends on network conditions. Anyway, let's check for both.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
[not found] ` <8E98F721-601F-436D-8F0A-5E399D8F7CAB@tarantool.org>
@ 2019-09-06 13:44 ` Alexander Turenko
2019-09-10 12:52 ` Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-09-06 13:44 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
> >> +--
> >> +-- gh-4138: check getaddrinfo() error and panic after that.
> >> +--
> >> +code=[[
> >> +local socket = require('socket')
> >> +local log = require('log')
> >> +local fio = require('fio')
> >> +
> >> +path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock')
> >> +unix_socket = socket('AF_UNIX', 'SOCK_DGRAM', 0)
> >> +unix_socket:bind('unix/', path)
> >> +
> >> +opt = string.format("syslog:server=non_exists_hostname:%s,identity=tarantool", path)
> >> +box.cfg{log = opt, log_nonblock=true}
> >
> > log_nonblock is not needed here, so it is better to remove it.
> Removed.
Still no.
> > box.cfg{log = 'syslog:server=non_exists_hostname:3301'} is enough, not need to
> > form a file path, no need identity, no need requiring socket, log and fio.
> >
> > The test passes even before the patch, so what it is intended to test? I
> > think we should write a test that verifies stderr output to find all log
> > messages we expect to appear in the case:
> >
> > Linux:
> >
> > | SystemError getaddrinfo: Temporary failure in name resolution: Input/output error
> > | SystemError syslog logger: Input/output error: Input/output error
> > | failed to initialize logging subsystem
> >
> > gai_strerror() message corresponds to EAI_AGAIN.
> >
> > Mac OS:
> >
> > | SystemError getaddrinfo: nodename nor servname provided, or not known: Input/output error
> > | SystemError syslog logger: Input/output error: Input/output error
> > | failed to initialize logging subsystem
> >
> > gai_strerror() message corresponds to EAI_NONAME.
> >
> > I propose to call ffi.C.gai_strerror() right from a test to form two
> > error messages and verify that the actual input match one of them.
> >
> > If it is hard to catch stderr, then let's proceed w/o this test. However
> > I think it is doable.
> >
> > I also propose to test error messages in the similar way (using
> > ffi.C.gai_strerror(GAI_AGAIN) and ffi.C.gai_strerror(GAI_NONAME)) in
> > test cases in second patch of the patchset.
> We discussed that with Vova and we decided, that it is too difficult.
> It is better, to waste not time for this test. Test for PANIC is enough.
I don't see any reason to add a test case that checks that tarantool
exits in the case: it was so before the commit and nothing is changed
except the error message. I propose to delete the test case.
> >> + /* gh-4138: Check getaddrinfo() error. */
> >> + isnt(coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1), 0,
> >> + "getaddrinfo error");
> >
> > I would say 'getaddrinfo retval' instead 'getaddrinfo error'.
I don't insist, but remind about that here if it was missed by a
mistake.
> >> + isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
> >> + "getaddrinfo error message");
> >> +
> >
> > I propose to verify the entire error message using
> > gai_strerror(GAI_AGAIN) and gai_strerror(GAI_NONAME)—just as proposed
> > above for a log message.
> I have not found the way, how to carry this macros/enum from the libc header to Lua.
It is possible to add them into socket.c, but anyway it is not worth to
do this just for a test. Let's check for certain error messages (not for
just 'getaddrinfo: ' prefix).
^ permalink raw reply [flat|nested] 19+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-09-06 13:44 ` Alexander Turenko
@ 2019-09-10 12:52 ` Roman Khabibov
2019-11-01 15:19 ` [Tarantool-patches] " Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-09-10 12:52 UTC (permalink / raw)
To: tarantool-patches; +Cc: Alexander Turenko
> On Sep 6, 2019, at 16:44, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
>
>>>> +--
>>>> +-- gh-4138: check getaddrinfo() error and panic after that.
>>>> +--
>>>> +code=[[
>>>> +local socket = require('socket')
>>>> +local log = require('log')
>>>> +local fio = require('fio')
>>>> +
>>>> +path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock')
>>>> +unix_socket = socket('AF_UNIX', 'SOCK_DGRAM', 0)
>>>> +unix_socket:bind('unix/', path)
>>>> +
>>>> +opt = string.format("syslog:server=non_exists_hostname:%s,identity=tarantool", path)
>>>> +box.cfg{log = opt, log_nonblock=true}
>>>
>>> log_nonblock is not needed here, so it is better to remove it.
>> Removed.
>
> Still no.
>
>>> box.cfg{log = 'syslog:server=non_exists_hostname:3301'} is enough, not need to
>>> form a file path, no need identity, no need requiring socket, log and fio.
>>>
>>> The test passes even before the patch, so what it is intended to test? I
>>> think we should write a test that verifies stderr output to find all log
>>> messages we expect to appear in the case:
>>>
>>> Linux:
>>>
>>> | SystemError getaddrinfo: Temporary failure in name resolution: Input/output error
>>> | SystemError syslog logger: Input/output error: Input/output error
>>> | failed to initialize logging subsystem
>>>
>>> gai_strerror() message corresponds to EAI_AGAIN.
>>>
>>> Mac OS:
>>>
>>> | SystemError getaddrinfo: nodename nor servname provided, or not known: Input/output error
>>> | SystemError syslog logger: Input/output error: Input/output error
>>> | failed to initialize logging subsystem
>>>
>>> gai_strerror() message corresponds to EAI_NONAME.
>>>
>>> I propose to call ffi.C.gai_strerror() right from a test to form two
>>> error messages and verify that the actual input match one of them.
>>>
>>> If it is hard to catch stderr, then let's proceed w/o this test. However
>>> I think it is doable.
>>>
>>> I also propose to test error messages in the similar way (using
>>> ffi.C.gai_strerror(GAI_AGAIN) and ffi.C.gai_strerror(GAI_NONAME)) in
>>> test cases in second patch of the patchset.
>> We discussed that with Vova and we decided, that it is too difficult.
>> It is better, to waste not time for this test. Test for PANIC is enough.
>
> I don't see any reason to add a test case that checks that tarantool
> exits in the case: it was so before the commit and nothing is changed
> except the error message. I propose to delete the test case.
Ok. Removed.
>>>> + /* gh-4138: Check getaddrinfo() error. */
>>>> + isnt(coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1), 0,
>>>> + "getaddrinfo error");
>>>
>>> I would say 'getaddrinfo retval' instead 'getaddrinfo error'.
>
> I don't insist, but remind about that here if it was missed by a
> mistake.
+ /* gh-4138: Check getaddrinfo() retval. */
+ rc = coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1);
+ isnt(rc, 0, "getaddrinfo retval");
+ isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
+ "getaddrinfo error message");
+
>>>> + isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
>>>> + "getaddrinfo error message");
>>>> +
>>>
>>> I propose to verify the entire error message using
>>> gai_strerror(GAI_AGAIN) and gai_strerror(GAI_NONAME)—just as proposed
>>> above for a log message.
>> I have not found the way, how to carry this macros/enum from the libc header to Lua.
>
> It is possible to add them into socket.c, but anyway it is not worth to
> do this just for a test. Let's check for certain error messages (not for
> just 'getaddrinfo: ' prefix).
commit 3e973c2203131ab663bdf1154be73ff6592568ea
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 30 15:39:21 2019 +0300
coiox/say: fix getaddrinfo error handling on macOS
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on macOS, because they are greater than 0 on
macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
macOS).
Note: diag_log() in say.c was added, because otherwise it will be
hid by the following diagnostic and then say that it should
be handler in a better way after #1148. Also, two diag_set() in
syslog_connect_unix() was added to avoid asserts in this
diag_log().
Need for #4138
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index 908b336ed..83f669d05 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
return -1; /* timed out or cancelled */
/* Task finished */
- if (task->rc < 0) {
+ if (task->rc != 0) {
/* getaddrinfo() failed */
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
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");
return -1;
+ }
struct sockaddr_un un;
memset(&un, 0, sizeof(un));
snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
un.sun_family = AF_UNIX;
if (connect(fd, (struct sockaddr *) &un, sizeof(un)) != 0) {
close(fd);
+ diag_set(SystemError, "connect");
return -1;
}
return fd;
@@ -506,7 +509,7 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
@@ -593,6 +596,11 @@ log_syslog_init(struct log *log, const char *init_str)
say_free_syslog_opts(&opts);
log->fd = log_syslog_connect(log);
if (log->fd < 0) {
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
/* syslog indent is freed in atexit(). */
diag_set(SystemError, "syslog logger: %s", strerror(errno));
return -1;
diff --git a/test/unit/coio.cc b/test/unit/coio.cc
index bb8bd7131..3758c8857 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,12 @@ test_getaddrinfo(void)
is(rc, 0, "getaddrinfo");
freeaddrinfo(i);
+ /* gh-4138: Check getaddrinfo() retval. */
+ rc = coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1);
+ isnt(rc, 0, "getaddrinfo retval");
+ isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
+ "getaddrinfo error message");
+
/*
* gh-4209: 0 timeout should not be a special value and
* detach a task. Before a fix it led to segfault
diff --git a/test/unit/coio.result b/test/unit/coio.result
index 5019fa48a..90b567140 100644
--- a/test/unit/coio.result
+++ b/test/unit/coio.result
@@ -7,6 +7,8 @@
# call done with res 0
*** test_call_f: done ***
*** test_getaddrinfo ***
-1..1
+1..3
ok 1 - getaddrinfo
+ok 2 - getaddrinfo retval
+ok 3 - getaddrinfo error message
*** test_getaddrinfo: done ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-09-10 12:52 ` Roman Khabibov
@ 2019-11-01 15:19 ` Alexander Turenko
2019-11-21 17:28 ` [Tarantool-patches] [tarantool-patches] " Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-11-01 15:19 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches, tarantool-patches
> coiox/say: fix getaddrinfo error handling on macOS
Typo: coiox -> coio.
> >>>> + isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
> >>>> + "getaddrinfo error message");
> >>>> +
> >>>
> >>> I propose to verify the entire error message using
> >>> gai_strerror(GAI_AGAIN) and gai_strerror(GAI_NONAME)—just as proposed
> >>> above for a log message.
> >> I have not found the way, how to carry this macros/enum from the libc header to Lua.
> >
> > It is possible to add them into socket.c, but anyway it is not worth to
> > do this just for a test. Let's check for certain error messages (not for
> > just 'getaddrinfo: ' prefix).
Again: let's check for certain error messages (not for just
'getaddrinfo: ' prefix).
The patch looks okay other then this.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-11-01 15:19 ` [Tarantool-patches] " Alexander Turenko
@ 2019-11-21 17:28 ` Roman Khabibov
2019-12-08 19:48 ` Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-11-21 17:28 UTC (permalink / raw)
To: tarantool-patches
> On Nov 1, 2019, at 18:19, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
>
>> coiox/say: fix getaddrinfo error handling on macOS
>
> Typo: coiox -> coio.
Ok.
>>>>>> + isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL,
>>>>>> + "getaddrinfo error message");
>>>>>> +
>>>>>
>>>>> I propose to verify the entire error message using
>>>>> gai_strerror(GAI_AGAIN) and gai_strerror(GAI_NONAME)—just as proposed
>>>>> above for a log message.
>>>> I have not found the way, how to carry this macros/enum from the libc header to Lua.
>>>
>>> It is possible to add them into socket.c, but anyway it is not worth to
>>> do this just for a test. Let's check for certain error messages (not for
>>> just 'getaddrinfo: ' prefix).
>
> Again: let's check for certain error messages (not for just
> 'getaddrinfo: ' prefix).
I have changed the timeout to 15768000000, because last->errmsg was ’timed out’.
@@ -72,14 +72,28 @@ static void
test_getaddrinfo(void)
{
header();
- plan(1);
+ plan(3);
const char *host = "127.0.0.1";
const char *port = "3333";
struct addrinfo *i;
/* NULL hints should work. It is a standard. */
int rc = coio_getaddrinfo(host, port, NULL, &i, 1);
is(rc, 0, "getaddrinfo");
- freeaddrinfo(i);
+
+ /* gh-4138: Check getaddrinfo() retval. */
+ 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";
+ bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
+ strcmp(errmsg, exp_errmsg_2) == 0 ||
+ strcmp(errmsg, exp_errmsg_3) == 0;
+ is(is_match_with_exp, true, "getaddrinfo error message");
/*
* gh-4209: 0 timeout should not be a special value and
> The patch looks okay other then this.
commit d9c23fde4c7c1fa2979954be83c1a2f6f8407347
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 30 15:39:21 2019 +0300
coio/say: fix getaddrinfo error handling on macOS
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on macOS, because they are greater than 0 on
macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
macOS).
Note: diag_log() in say.c was added, because otherwise it will be
hid by the following diagnostic and then say that it should
be handler in a better way after #1148. Also, two diag_set() in
syslog_connect_unix() was added to avoid asserts in this
diag_log().
Need for #4138
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index 908b336ed..83f669d05 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
return -1; /* timed out or cancelled */
/* Task finished */
- if (task->rc < 0) {
+ if (task->rc != 0) {
/* getaddrinfo() failed */
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
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");
return -1;
+ }
struct sockaddr_un un;
memset(&un, 0, sizeof(un));
snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
un.sun_family = AF_UNIX;
if (connect(fd, (struct sockaddr *) &un, sizeof(un)) != 0) {
close(fd);
+ diag_set(SystemError, "connect");
return -1;
}
return fd;
@@ -506,7 +509,7 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
@@ -593,6 +596,11 @@ log_syslog_init(struct log *log, const char *init_str)
say_free_syslog_opts(&opts);
log->fd = log_syslog_connect(log);
if (log->fd < 0) {
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
/* syslog indent is freed in atexit(). */
diag_set(SystemError, "syslog logger: %s", strerror(errno));
return -1;
diff --git a/test/unit/coio.cc b/test/unit/coio.cc
index bb8bd7131..d5cbd2fb9 100644
--- a/test/unit/coio.cc
+++ b/test/unit/coio.cc
@@ -72,14 +72,28 @@ static void
test_getaddrinfo(void)
{
header();
- plan(1);
+ plan(3);
const char *host = "127.0.0.1";
const char *port = "3333";
struct addrinfo *i;
/* NULL hints should work. It is a standard. */
int rc = coio_getaddrinfo(host, port, NULL, &i, 1);
is(rc, 0, "getaddrinfo");
- freeaddrinfo(i);
+
+ /* gh-4138: Check getaddrinfo() retval. */
+ 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";
+ bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
+ strcmp(errmsg, exp_errmsg_2) == 0 ||
+ strcmp(errmsg, exp_errmsg_3) == 0;
+ is(is_match_with_exp, true, "getaddrinfo error message");
/*
* gh-4209: 0 timeout should not be a special value and
diff --git a/test/unit/coio.result b/test/unit/coio.result
index 5019fa48a..90b567140 100644
--- a/test/unit/coio.result
+++ b/test/unit/coio.result
@@ -7,6 +7,8 @@
# call done with res 0
*** test_call_f: done ***
*** test_getaddrinfo ***
-1..1
+1..3
ok 1 - getaddrinfo
+ok 2 - getaddrinfo retval
+ok 3 - getaddrinfo error message
*** test_getaddrinfo: done ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-11-21 17:28 ` [Tarantool-patches] [tarantool-patches] " Roman Khabibov
@ 2019-12-08 19:48 ` Alexander Turenko
2019-12-10 16:25 ` Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-12-08 19:48 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
> @@ -72,14 +72,28 @@ static void
> test_getaddrinfo(void)
> {
> header();
> - plan(1);
> + plan(3);
> const char *host = "127.0.0.1";
> const char *port = "3333";
> struct addrinfo *i;
> /* NULL hints should work. It is a standard. */
> int rc = coio_getaddrinfo(host, port, NULL, &i, 1);
> is(rc, 0, "getaddrinfo");
> - freeaddrinfo(i);
Why do you remove freeing of the previous result? It will lead to a leak
and so will require extra work to run unit tests under ASAN or Valgrind.
> +
> + /* gh-4138: Check getaddrinfo() retval. */
Nit: retval and diagnostics area. You changed the code and the comment
becomes inaccurate.
> + 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";
> + bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
> + strcmp(errmsg, exp_errmsg_2) == 0 ||
> + strcmp(errmsg, exp_errmsg_3) == 0;
> + is(is_match_with_exp, true, "getaddrinfo error message");
>
> /*
> * gh-4209: 0 timeout should not be a special value and
>
> > The patch looks okay other then this.
>
> commit d9c23fde4c7c1fa2979954be83c1a2f6f8407347
> Author: Roman Khabibov <roman.habibov@tarantool.org>
> Date: Tue Jul 30 15:39:21 2019 +0300
>
> coio/say: fix getaddrinfo error handling on macOS
>
> Before this patch, branch when getaddrinfo() returns error codes
> couldn't be reached on macOS, because they are greater than 0 on
> macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
> macOS).
>
> Note: diag_log() in say.c was added, because otherwise it will be
> hid by the following diagnostic and then say that it should
> be handler in a better way after #1148. Also, two diag_set() in
'and then say' -- looks as copy-paste from review notes.
Typo: handler -> handled.
> syslog_connect_unix() was added to avoid asserts in this
> diag_log().
>
> Need for #4138
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-08 19:48 ` Alexander Turenko
@ 2019-12-10 16:25 ` Roman Khabibov
2019-12-18 15:01 ` Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-12-10 16:25 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Hi! Thanks for the review.
> On Dec 8, 2019, at 22:48, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
>
>> @@ -72,14 +72,28 @@ static void
>> test_getaddrinfo(void)
>> {
>> header();
>> - plan(1);
>> + plan(3);
>> const char *host = "127.0.0.1";
>> const char *port = "3333";
>> struct addrinfo *i;
>> /* NULL hints should work. It is a standard. */
>> int rc = coio_getaddrinfo(host, port, NULL, &i, 1);
>> is(rc, 0, "getaddrinfo");
>> - freeaddrinfo(i);
>
> Why do you remove freeing of the previous result? It will lead to a leak
> and so will require extra work to run unit tests under ASAN or Valgrind.
Sorry. I did it for debug and forgot to return this line.
@@ -81,6 +81,24 @@ 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";
+ bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
+ strcmp(errmsg, exp_errmsg_2) == 0 ||
+ strcmp(errmsg, exp_errmsg_3) == 0;
+ is(is_match_with_exp, true, "getaddrinfo error message");
+
>> +
>> + /* gh-4138: Check getaddrinfo() retval. */
>
> Nit: retval and diagnostics area. You changed the code and the comment
> becomes inaccurate.
>
>> + 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";
>> + bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
>> + strcmp(errmsg, exp_errmsg_2) == 0 ||
>> + strcmp(errmsg, exp_errmsg_3) == 0;
>> + is(is_match_with_exp, true, "getaddrinfo error message");
>>
>> /*
>> * gh-4209: 0 timeout should not be a special value and
>>
>>> The patch looks okay other then this.
>>
>> commit d9c23fde4c7c1fa2979954be83c1a2f6f8407347
>> Author: Roman Khabibov <roman.habibov@tarantool.org>
>> Date: Tue Jul 30 15:39:21 2019 +0300
>>
>> coio/say: fix getaddrinfo error handling on macOS
>>
>> Before this patch, branch when getaddrinfo() returns error codes
>> couldn't be reached on macOS, because they are greater than 0 on
>> macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
>> macOS).
>>
>> Note: diag_log() in say.c was added, because otherwise it will be
>> hid by the following diagnostic and then say that it should
>> be handler in a better way after #1148. Also, two diag_set() in
>
> 'and then say' -- looks as copy-paste from review notes.
>
> Typo: handler -> handled.
Done.
>> syslog_connect_unix() was added to avoid asserts in this
>> diag_log().
>>
>> Need for #4138
commit ff683ad948dd9404221979f399b155c63aa88e5a
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 30 15:39:21 2019 +0300
coio/say: fix getaddrinfo error handling on macOS
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on macOS, because they are greater than 0 on
macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
macOS).
Note: diag_log() in say.c was added, because otherwise it will be
hid by the following diagnostic and it should be handled in a
better way after #1148. Also, two diag_set() in
syslog_connect_unix() was added to avoid asserts in this
diag_log().
Need for #4138
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index 908b336ed..83f669d05 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
return -1; /* timed out or cancelled */
/* Task finished */
- if (task->rc < 0) {
+ if (task->rc != 0) {
/* getaddrinfo() failed */
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
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");
return -1;
+ }
struct sockaddr_un un;
memset(&un, 0, sizeof(un));
snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
un.sun_family = AF_UNIX;
if (connect(fd, (struct sockaddr *) &un, sizeof(un)) != 0) {
close(fd);
+ diag_set(SystemError, "connect");
return -1;
}
return fd;
@@ -506,7 +509,7 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
@@ -593,6 +596,11 @@ log_syslog_init(struct log *log, const char *init_str)
say_free_syslog_opts(&opts);
log->fd = log_syslog_connect(log);
if (log->fd < 0) {
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
/* syslog indent is freed in atexit(). */
diag_set(SystemError, "syslog logger: %s", strerror(errno));
return -1;
diff --git a/test/unit/coio.cc b/test/unit/coio.cc
index bb8bd7131..aa38f5a99 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,24 @@ 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";
+ bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
+ strcmp(errmsg, exp_errmsg_2) == 0 ||
+ strcmp(errmsg, exp_errmsg_3) == 0;
+ is(is_match_with_exp, true, "getaddrinfo error message");
+
/*
* gh-4209: 0 timeout should not be a special value and
* detach a task. Before a fix it led to segfault
diff --git a/test/unit/coio.result b/test/unit/coio.result
index 5019fa48a..90b567140 100644
--- a/test/unit/coio.result
+++ b/test/unit/coio.result
@@ -7,6 +7,8 @@
# call done with res 0
*** test_call_f: done ***
*** test_getaddrinfo ***
-1..1
+1..3
ok 1 - getaddrinfo
+ok 2 - getaddrinfo retval
+ok 3 - getaddrinfo error message
*** test_getaddrinfo: done ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-10 16:25 ` Roman Khabibov
@ 2019-12-18 15:01 ` Alexander Turenko
2019-12-21 17:50 ` Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-12-18 15:01 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
Only one comment.
unit/coio fails on FreeBSD:
[006] Test failed! Result content mismatch:
[006] --- unit/coio.result Wed Dec 11 09:58:11 2019
[006] +++ unit/coio.reject Wed Sep 18 08:51:42 2019
[006] @@ -1,3 +1,6 @@
[006] +# Failed test 'getaddrinfo error message'
[006] +# in /home/vagrant/tarantool/test/unit/coio.cc at line 100
[006] +# Looks like you failed 1 test of 3 run.
[006] *** stat_timeout_test ***
[006] *** stat_timeout_test: done ***
[006] *** stat_notify_test ***
[006] @@ -10,5 +13,5 @@
[006] 1..3
[006] ok 1 - getaddrinfo
[006] ok 2 - getaddrinfo retval
[006] -ok 3 - getaddrinfo error message
[006] +not ok 3 - getaddrinfo error message
[006] *** test_getaddrinfo: done ***
https://gitlab.com/tarantool/tarantool/-/jobs/375844818
Please, also push a branch with -full-ci postfix: this will run the
whole testing matrix on the branch in GitLab-CI. It worth to verify this
just in case.
WBR, Alexander Turenko.
On Tue, Dec 10, 2019 at 07:25:05PM +0300, Roman Khabibov wrote:
> Hi! Thanks for the review.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-18 15:01 ` Alexander Turenko
@ 2019-12-21 17:50 ` Roman Khabibov
2019-12-23 12:56 ` Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-12-21 17:50 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
> On Dec 18, 2019, at 18:01, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
>
> Only one comment.
>
> unit/coio fails on FreeBSD:
Fixed.
> [006] Test failed! Result content mismatch:
> [006] --- unit/coio.result Wed Dec 11 09:58:11 2019
> [006] +++ unit/coio.reject Wed Sep 18 08:51:42 2019
> [006] @@ -1,3 +1,6 @@
> [006] +# Failed test 'getaddrinfo error message'
> [006] +# in /home/vagrant/tarantool/test/unit/coio.cc at line 100
> [006] +# Looks like you failed 1 test of 3 run.
> [006] *** stat_timeout_test ***
> [006] *** stat_timeout_test: done ***
> [006] *** stat_notify_test ***
> [006] @@ -10,5 +13,5 @@
> [006] 1..3
> [006] ok 1 - getaddrinfo
> [006] ok 2 - getaddrinfo retval
> [006] -ok 3 - getaddrinfo error message
> [006] +not ok 3 - getaddrinfo error message
> [006] *** test_getaddrinfo: done ***
>
> https://gitlab.com/tarantool/tarantool/-/jobs/375844818
>
> Please, also push a branch with -full-ci postfix: this will run the
> whole testing matrix on the branch in GitLab-CI. It worth to verify this
> just in case.
>
> WBR, Alexander Turenko.
>
> On Tue, Dec 10, 2019 at 07:25:05PM +0300, Roman Khabibov wrote:
>> Hi! Thanks for the review.
commit ea8478bc3363cb9278d40f412dfccd9a43e70e0e
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 30 15:39:21 2019 +0300
coio/say: fix getaddrinfo error handling on macOS
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on macOS, because they are greater than 0 on
macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
macOS).
Note: diag_log() in say.c was added, because otherwise it will be
hid by the following diagnostic and it should be handled in a
better way after #1148. Also, two diag_set() in
syslog_connect_unix() was added to avoid asserts in this
diag_log().
Need for #4138
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index 908b336ed..83f669d05 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
return -1; /* timed out or cancelled */
/* Task finished */
- if (task->rc < 0) {
+ if (task->rc != 0) {
/* getaddrinfo() failed */
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
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");
return -1;
+ }
struct sockaddr_un un;
memset(&un, 0, sizeof(un));
snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
un.sun_family = AF_UNIX;
if (connect(fd, (struct sockaddr *) &un, sizeof(un)) != 0) {
close(fd);
+ diag_set(SystemError, "connect");
return -1;
}
return fd;
@@ -506,7 +509,7 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
@@ -593,6 +596,11 @@ log_syslog_init(struct log *log, const char *init_str)
say_free_syslog_opts(&opts);
log->fd = log_syslog_connect(log);
if (log->fd < 0) {
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
/* syslog indent is freed in atexit(). */
diag_set(SystemError, "syslog logger: %s", strerror(errno));
return -1;
diff --git a/test/unit/coio.cc b/test/unit/coio.cc
index bb8bd7131..1b4ffa32a 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,27 @@ 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";
+ 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;
+ is(is_match_with_exp, true, "getaddrinfo error message");
+
/*
* gh-4209: 0 timeout should not be a special value and
* detach a task. Before a fix it led to segfault
diff --git a/test/unit/coio.result b/test/unit/coio.result
index 5019fa48a..90b567140 100644
--- a/test/unit/coio.result
+++ b/test/unit/coio.result
@@ -7,6 +7,8 @@
# call done with res 0
*** test_call_f: done ***
*** test_getaddrinfo ***
-1..1
+1..3
ok 1 - getaddrinfo
+ok 2 - getaddrinfo retval
+ok 3 - getaddrinfo error message
*** test_getaddrinfo: done ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-21 17:50 ` Roman Khabibov
@ 2019-12-23 12:56 ` Alexander Turenko
2019-12-23 13:38 ` Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-12-23 12:56 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
LGTM. Please, proceed with the next reviewer (Sergey O.).
> > Please, also push a branch with -full-ci postfix: this will run the
> > whole testing matrix on the branch in GitLab-CI. It worth to verify this
> > just in case.
I still think that you should do that.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-23 12:56 ` Alexander Turenko
@ 2019-12-23 13:38 ` Alexander Turenko
2019-12-26 17:29 ` Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-12-23 13:38 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
On Mon, Dec 23, 2019 at 03:56:58PM +0300, Alexander Turenko wrote:
> LGTM. Please, proceed with the next reviewer (Sergey O.).
>
> > > Please, also push a branch with -full-ci postfix: this will run the
> > > whole testing matrix on the branch in GitLab-CI. It worth to verify this
> > > just in case.
>
> I still think that you should do that.
I run tests and got EAI_AGAIN. Let's add it too to the list. See
https://lists.tarantool.org/pipermail/tarantool-patches/2019-December/013260.html
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-23 13:38 ` Alexander Turenko
@ 2019-12-26 17:29 ` Roman Khabibov
2019-12-26 21:05 ` Alexander Turenko
0 siblings, 1 reply; 19+ messages in thread
From: Roman Khabibov @ 2019-12-26 17:29 UTC (permalink / raw)
To: Sergey Ostanevich; +Cc: tarantool-patches
> On Dec 23, 2019, at 16:38, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
>
> On Mon, Dec 23, 2019 at 03:56:58PM +0300, Alexander Turenko wrote:
>> LGTM. Please, proceed with the next reviewer (Sergey O.).
>>
>>>> Please, also push a branch with -full-ci postfix: this will run the
>>>> whole testing matrix on the branch in GitLab-CI. It worth to verify this
>>>> just in case.
>>
>> I still think that you should do that.
>
> I run tests and got EAI_AGAIN. Let's add it too to the list. See
> https://lists.tarantool.org/pipermail/tarantool-patches/2019-December/013260.html
commit 4ac352b12ee701898042808408eadcd24caf5bb9
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 30 15:39:21 2019 +0300
coio/say: fix getaddrinfo error handling on macOS
Before this patch, branch when getaddrinfo() returns error codes
couldn't be reached on macOS, because they are greater than 0 on
macOS (assumption "rc < 0" in commit ea1da04 is incorrect for
macOS).
Note: diag_log() in say.c was added, because otherwise it will be
hid by the following diagnostic and it should be handled in a
better way after #1148. Also, two diag_set() in
syslog_connect_unix() was added to avoid asserts in this
diag_log().
Need for #4138
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index 908b336ed..83f669d05 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port,
return -1; /* timed out or cancelled */
/* Task finished */
- if (task->rc < 0) {
+ if (task->rc != 0) {
/* getaddrinfo() failed */
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
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");
return -1;
+ }
struct sockaddr_un un;
memset(&un, 0, sizeof(un));
snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
un.sun_family = AF_UNIX;
if (connect(fd, (struct sockaddr *) &un, sizeof(un)) != 0) {
close(fd);
+ diag_set(SystemError, "connect");
return -1;
}
return fd;
@@ -506,7 +509,7 @@ syslog_connect_remote(const char *server_address)
hints.ai_protocol = IPPROTO_UDP;
ret = getaddrinfo(remote, portnum, &hints, &inf);
- if (ret < 0) {
+ if (ret != 0) {
errno = EIO;
diag_set(SystemError, "getaddrinfo: %s",
gai_strerror(ret));
@@ -593,6 +596,11 @@ log_syslog_init(struct log *log, const char *init_str)
say_free_syslog_opts(&opts);
log->fd = log_syslog_connect(log);
if (log->fd < 0) {
+ /*
+ * We need to log a diagnostics here until stacked
+ * diagnostics will be implemented (#1148).
+ */
+ diag_log();
/* syslog indent is freed in atexit(). */
diag_set(SystemError, "syslog logger: %s", strerror(errno));
return -1;
diff --git a/test/unit/coio.cc b/test/unit/coio.cc
index bb8bd7131..22c2a9226 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,30 @@ 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";
+ 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;
+ is(is_match_with_exp, true, "getaddrinfo error message");
+
/*
* gh-4209: 0 timeout should not be a special value and
* detach a task. Before a fix it led to segfault
diff --git a/test/unit/coio.result b/test/unit/coio.result
index 5019fa48a..90b567140 100644
--- a/test/unit/coio.result
+++ b/test/unit/coio.result
@@ -7,6 +7,8 @@
# call done with res 0
*** test_call_f: done ***
*** test_getaddrinfo ***
-1..1
+1..3
ok 1 - getaddrinfo
+ok 2 - getaddrinfo retval
+ok 3 - getaddrinfo error message
*** test_getaddrinfo: done ***
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-26 17:29 ` Roman Khabibov
@ 2019-12-26 21:05 ` Alexander Turenko
2019-12-27 13:01 ` Roman Khabibov
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Turenko @ 2019-12-26 21:05 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
> > I run tests and got EAI_AGAIN. Let's add it too to the list. See
> > https://lists.tarantool.org/pipermail/tarantool-patches/2019-December/013260.html
> + /*
> + * 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";
Linux:
$ gcc -Wall -Wextra -x c <(echo -e '#include <sys/types.h>\n#include <sys/socket.h>\n#include <netdb.h>\n#include <stdio.h>\nint main() { printf("%s\\n", gai_strerror(EAI_AGAIN)); return 0; }') && ./a.out; rm a.out
Temporary failure in name resolution
Mac OS:
$ clang -Wall -Wextra -x c <(echo -e '#include <sys/types.h>\n#include <sys/socket.h>\n#include <netdb.h>\n#include <stdio.h>\nint main() { printf("%s\\n", gai_strerror(EAI_AGAIN)); return 0; }') && ./a.out; rm a.out
Temporary failure in name resolution
FreeBSD:
% printf '#include <sys/types.h>\n#include <sys/socket.h>\n#include <netdb.h>\n#include <stdio.h>\nint main() { printf("%%s\\n", gai_strerror(EAI_AGAIN)); return 0; }' > tmp.c && gcc -Wall -Wextra -x c tmp.c && ./a.out ; rm a.out tmp.c
Name could not be resolved at this time
"temporary failure in name resolution" does not match any.
> + 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;
> + is(is_match_with_exp, true, "getaddrinfo error message");
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
2019-12-26 21:05 ` Alexander Turenko
@ 2019-12-27 13:01 ` Roman Khabibov
0 siblings, 0 replies; 19+ messages in thread
From: Roman Khabibov @ 2019-12-27 13:01 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Hi!
> Linux:
>
> $ gcc -Wall -Wextra -x c <(echo -e '#include <sys/types.h>\n#include <sys/socket.h>\n#include <netdb.h>\n#include <stdio.h>\nint main() { printf("%s\\n", gai_strerror(EAI_AGAIN)); return 0; }') && ./a.out; rm a.out
> Temporary failure in name resolution
>
> Mac OS:
>
> $ clang -Wall -Wextra -x c <(echo -e '#include <sys/types.h>\n#include <sys/socket.h>\n#include <netdb.h>\n#include <stdio.h>\nint main() { printf("%s\\n", gai_strerror(EAI_AGAIN)); return 0; }') && ./a.out; rm a.out
> Temporary failure in name resolution
>
> FreeBSD:
>
> % printf '#include <sys/types.h>\n#include <sys/socket.h>\n#include <netdb.h>\n#include <stdio.h>\nint main() { printf("%%s\\n", gai_strerror(EAI_AGAIN)); return 0; }' > tmp.c && gcc -Wall -Wextra -x c tmp.c && ./a.out ; rm a.out tmp.c
> Name could not be resolved at this time
>
> "temporary failure in name resolution" does not match any.
Fixed.
I looked here.
https://www.freebsd.org/cgi/man.cgi?query=gai_strerror&sektion=3&manpath=FreeBSD+5.4-RELEASE
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-12-27 13:01 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25 13:38 [tarantool-patches] [PATCH v2 0/2] take getaddrinfo() errors into account Roman Khabibov
2019-06-25 13:38 ` [tarantool-patches] [PATCH v2 2/2] say: " Roman Khabibov
2019-07-23 14:52 ` [tarantool-patches] " Alexander Turenko
2019-08-05 13:32 ` Roman Khabibov
2019-08-28 21:34 ` Alexander Turenko
2019-08-29 0:51 ` Alexander Turenko
[not found] ` <8E98F721-601F-436D-8F0A-5E399D8F7CAB@tarantool.org>
2019-09-06 13:44 ` Alexander Turenko
2019-09-10 12:52 ` Roman Khabibov
2019-11-01 15:19 ` [Tarantool-patches] " Alexander Turenko
2019-11-21 17:28 ` [Tarantool-patches] [tarantool-patches] " Roman Khabibov
2019-12-08 19:48 ` Alexander Turenko
2019-12-10 16:25 ` Roman Khabibov
2019-12-18 15:01 ` Alexander Turenko
2019-12-21 17:50 ` Roman Khabibov
2019-12-23 12:56 ` Alexander Turenko
2019-12-23 13:38 ` Alexander Turenko
2019-12-26 17:29 ` Roman Khabibov
2019-12-26 21:05 ` Alexander Turenko
2019-12-27 13:01 ` Roman Khabibov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox