Re: [PATCH] builtin/: add UNLEAKs
- Date: Mon, 2 Oct 2017 02:25:28 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH] builtin/: add UNLEAKs
On Sun, Oct 01, 2017 at 07:42:08PM +0200, Martin Ågren wrote:
> Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the
> variables in the same order as we've declared them. While addressing
> `msg` in builtin/tag.c, convert the existing `strbuf_release()` calls as
It might have raised Junio's eyebrows less to say something like:
...convert the existing strbuf_release() calls as well (they're not
wrong, but they also accomplish nothing and create an inconsistency
with the UNLEAKed variables).
That aside, the patch looks good.
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 3345a0d16..2daa4412a 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1298,6 +1298,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> + UNLEAK(opts);
> if (opts.patch_mode || opts.pathspec.nr)
> return checkout_paths(&opts, new.name);
Seeing hunks like this makes me happy with the UNLEAK() solution. It
would have been a real pain to do this via actual freeing.