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@dev.tarantool.org
Subject: Re: [Tarantool-patches] [tarantool-patches] [PATCH v2 2/2] say: take getaddrinfo() errors into account
Date: Sun, 8 Dec 2019 22:48:24 +0300	[thread overview]
Message-ID: <20191208194823.krw4hc4lm7bhxqrk@tkn_work_nb> (raw)
In-Reply-To: <E1252366-59AF-47C1-AB36-F24A2A1DF4AF@tarantool.org>

> @@ -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

  reply	other threads:[~2019-12-08 19:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-25 13:38 [tarantool-patches] [PATCH v2 0/2] " 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 [this message]
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

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=20191208194823.krw4hc4lm7bhxqrk@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] [PATCH v2 2/2] say: take getaddrinfo() errors into account' \
    /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