Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: Aleksandr Lyapunov <alyapunov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] Fix wrong make_scoped_guard usage
Date: Thu, 9 Jul 2020 10:24:58 +0000	[thread overview]
Message-ID: <20200709102457.GA814@tarantool.org> (raw)
In-Reply-To: <1594210027-332-2-git-send-email-alyapunov@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
> 

  parent reply	other threads:[~2020-07-09 10:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-08 12:07 [Tarantool-patches] [PATCH] Fix " Aleksandr Lyapunov
2020-07-08 12:07 ` [Tarantool-patches] [PATCH] Fix wrong " Aleksandr Lyapunov
2020-07-08 12:15   ` Timur Safin
2020-07-08 12:16   ` Cyrill Gorcunov
2020-07-09 10:24   ` Nikita Pettik [this message]
2020-07-08 12:08 ` [Tarantool-patches] [PATCH] Fix " Aleksandr Lyapunov
2020-07-13 13:40 ` Kirill Yukhin

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=20200709102457.GA814@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=alyapunov@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] Fix wrong make_scoped_guard usage' \
    /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