Re: [PATCH 05/11] builtin/repack.c: avoid leaking child arguments
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 21, 2021, 16:37 UTC
- Message-ID
- <xmqqmtn2mj0s.fsf@gitster.g>
- In-Reply-To
- <a68c77c00638bef7e8ce88929015a587d2d1b6f8.1634787555.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 11 quoted lines
> `git repack` invokes a handful of child processes: one to write the > actual pack, and optionally ones to repack promisor objects and update > the MIDX. > > In none of these cases do we bother to call child_process_clear(), which > frees the memory associated with each child's arguments and environment. > > In order to do so, tweak each function that spawns a child process to > have a `cleanup` label that we always visit before returning from each > function. Then, make sure that we call child_process_clear() as a part > of that label.
I have a slight aversion against the fact that this patch makes us call child_process_clear(), when we know we may have called finish_command(). Callers of finish_command() in other codepaths rely on the fact that finish_command() calls child_process_clear(), which means that because of this change, we are now constrained to keep child_process_clear() idempotent?