Re: [PATCH v2 01/12] commit-graph: add 'verify' subcommand

From: Martin Ågren <martin.agren@gmail.com>
Date: May 12, 2018, 13:31 UTC
Message-ID: <CAN0heSotYYgrwMLT1yyFVk4VEZ4OQKF_Xwm9-wZosJ6_-gAVBg@mail.gmail.com>
In-Reply-To: <20180511211504.79877-2-dstolee@microsoft.com>

On 11 May 2018 at 23:15, Derrick Stolee <dstolee@microsoft.com> wrote:

Show 7 quoted lines

>         graph_name = get_commit_graph_filename(opts.obj_dir);
>         graph = load_commit_graph_one(graph_name);
> +       FREE_AND_NULL(graph_name);
>
>         if (!graph)
>                 die("graph file %s does not exist", graph_name);
> -       FREE_AND_NULL(graph_name);

This is probably because of something I said, but this does not look correct. The `die()` would typically print "(null)" or segfault. If the `die()` means we don't free `graph_name`, that should be fine.

You're still leaking `graph` here (possibly nothing this patch should worry about) and in `graph_verify()`. UNLEAK-ing it immediately before calling `verify_commit_graph()` should be ok. I also think punting on this UNLEAK-business entirely would be ok; I was just a bit surprised to see one variable getting freed and the other one ignored.

Martin