From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (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 02DA5445320 for ; Thu, 9 Jul 2020 13:24:58 +0300 (MSK) Date: Thu, 9 Jul 2020 10:24:58 +0000 From: Nikita Pettik Message-ID: <20200709102457.GA814@tarantool.org> References: <1594210027-332-1-git-send-email-alyapunov@tarantool.org> <1594210027-332-2-git-send-email-alyapunov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1594210027-332-2-git-send-email-alyapunov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] Fix wrong make_scoped_guard usage List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aleksandr Lyapunov Cc: tarantool-patches@dev.tarantool.org On 08 Jul 15:07, Aleksandr Lyapunov wrote: > The common pitfall of using a lambda is wrong type of capture - > by value instead of by reference. The simple example is: > struct sequence_def *new_def = NULL; > auto def_guard = make_scoped_guard([=] { free(new_def); }); > // initialize new_def > The problem is that the lambda captures pointer by value, that > is NULL and will remain NULL in the lambda while new_def is > successfully initialized in function scope. > > The patch fixes the problem above and a couple of similar mistakes. > --- Agree with Cyrill that we could simply use &. Anyway LGTM, let's push this patch. > src/box/alter.cc | 4 ++-- > src/main.cc | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/box/alter.cc b/src/box/alter.cc > index bb42548..3109fb4 100644 > --- a/src/box/alter.cc > +++ b/src/box/alter.cc > @@ -3560,7 +3560,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) > * definition to support upgrade script. > */ > struct func_def *old_def = NULL, *new_def = NULL; > - auto guard = make_scoped_guard([=] { > + auto guard = make_scoped_guard([&old_def, &new_def] { > free(old_def); > free(new_def); > }); > @@ -4431,7 +4431,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) > struct tuple *new_tuple = stmt->new_tuple; > > struct sequence_def *new_def = NULL; > - auto def_guard = make_scoped_guard([=] { free(new_def); }); > + auto def_guard = make_scoped_guard([&new_def] { free(new_def); }); > > struct sequence *seq; > if (old_tuple == NULL && new_tuple != NULL) { /* INSERT */ > diff --git a/src/main.cc b/src/main.cc > index 2c96391..e622042 100644 > --- a/src/main.cc > +++ b/src/main.cc > @@ -729,7 +729,7 @@ main(int argc, char **argv) > /* Lua interpeter options, e.g. -e and -l */ > int optc = 0; > const char **optv = NULL; > - auto guard = make_scoped_guard([=]{ if (optc) free(optv); }); > + auto guard = make_scoped_guard([&optc, &optv]{ if (optc) free(optv); }); > > static struct option longopts[] = { > {"help", no_argument, 0, 'h'}, > -- > 2.7.4 >