Re: [PATCH 10/17] sequencer.c: fix "opts->strategy" leak in read_strategy_opts()

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Nov 4, 2022, 14:50 UTC
Message-ID: <b9f08244-36bc-b74f-ac1a-b665423bc2e7@dunelm.org.uk>
In-Reply-To: <patch-10.17-9c70bfa334e-20221103T164632Z-avarab@gmail.com>

On 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote:

> When "read_strategy_opts()" is called we may have populated the
> "opts->strategy" before, so we'll need to free() it to avoid leaking
> memory.

Where is the previous value coming from? I guess it may be the config but otherwise I'm confused.

> Along with the preceding commit this change various
> rebase-related tests pass.

Really? at a glance the previous patch looks unrelated and there are no tests marked as passing in this one.

Best Wishes

Phillip

Show 17 quoted lines

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   sequencer.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index e658df7e8ff..07d7062bfb8 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2894,6 +2894,7 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
>   	strbuf_reset(buf);
>   	if (!read_oneliner(buf, rebase_path_strategy(), 0))
>   		return;
> +	free(opts->strategy);
>   	opts->strategy = strbuf_detach(buf, NULL);
>   	if (!read_oneliner(buf, rebase_path_strategy_opts(), 0))
>   		return;