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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 20, 2024, 17:00 UTC
Message-ID: <xmqq4j6a45wr.fsf@gitster.g>
In-Reply-To: <747c9a76a288d6fbfad85203e0bccaaf4b6db282.1726484308.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> 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.