Subject: Re: [PATCH 10/23] builtin/pull: fix leaking "ff" option

From: Junio C Hamano <[email-masked]>
Date: Fri, 20 Sep 2024 10:00:04 -0700

Patrick Steinhardt <[email-masked]> writes:

> The `opt_ff` field gets populated either via `OPT_PASSTHRU` via
> `config_get_ff()` or when `--rebase` is passed. So we sometimes end up
> overriding the value in `opt_ff` with another value, but we do not free
> the old value, causing a memory leak.

Good eyes.

I have to wonder if it will come back and bite us again that
OPT_PASSTHRU does not pass through but creates a new copy, while
OPT_STRING borrows from the argv[].  I guess that we could check in
parse_options_check() if the address of the same variable is passed
to both OPT_PASSTHRU (giving it a piece of memory we are responsible
for freeing) and OPT_STRING (giving it a piece of memory borrowed
from argv[], and would trigger a failure if given to free()).

In an extremely longer run, I wonder if it becomes necessary for us
to also make OPT_STRING to make a copy (which means strings that
come from the configuration and from parsing the command line
arguments are all allocated ones that we are responsible to free).

Thanks.