From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99])
 (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 D358E469710
 for <tarantool-patches@dev.tarantool.org>;
 Sun,  3 May 2020 21:45:22 +0300 (MSK)
References: <20200428161137.20536-1-gorcunov@gmail.com>
 <20200428161137.20536-8-gorcunov@gmail.com>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <9bcf869e-05af-e1d1-2537-255f84a41c9e@tarantool.org>
Date: Sun, 3 May 2020 20:45:21 +0200
MIME-Version: 1.0
In-Reply-To: <20200428161137.20536-8-gorcunov@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH 07/17] recovery: hot_standby_f --
 don't throw exception
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Cyrill Gorcunov <gorcunov@gmail.com>, tml <tarantool-patches@dev.tarantool.org>

Thanks for the patch!

On 28/04/2020 18:11, Cyrill Gorcunov wrote:
> Prepare for transition to plain C.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  src/box/recovery.cc | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/src/box/recovery.cc b/src/box/recovery.cc
> index 1b78fc915..996966a77 100644
> --- a/src/box/recovery.cc
> +++ b/src/box/recovery.cc
> @@ -494,18 +494,15 @@ hot_standby_f(va_list ap)
>  	struct recovery *r = va_arg(ap, struct recovery *);
>  	struct xstream *stream = va_arg(ap, struct xstream *);
>  	bool scan_dir = true;
> +	int rc = 0;
>  
>  	ev_tstamp wal_dir_rescan_delay = va_arg(ap, ev_tstamp);
>  	fiber_set_user(fiber(), &admin_credentials);
>  
>  	struct wal_subscr ws;
> -	auto guard = make_scoped_guard([&]{
> -		wal_subscr_destroy(&ws);
> -	});
> -
>  	wal_subscr_create(&ws, r->wal_dir.dirname);
>  
> -	while (! fiber_is_cancelled()) {
> +	while (!fiber_is_cancelled()) {

I know that sometimes you may really want to change something,
that looks 'wrong' even when it is not related to the patch, but
please, lets better avoid such changes. They not only make the
patch bigger complicating the review, but also make it harder to
navigate the code in 'git blame', when you want to discover why
something was done and by whom.