Re: [PATCH 14/22] shallow: fix leaking members of `struct shallow_info`
- From
Toon claes <toon@iotcl.com>
- Date
- Aug 29, 2024, 14:16 UTC
- Message-ID
- <87ed67jtxy.fsf@iotcl.com>
- In-Reply-To
- <2a63030ff09f938d705c117406b501ecf81f67de.1724656120.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 29 quoted lines
> We do not free several struct members in `clear_shallow_info()`. Fix
> this to plug the resulting leaks.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> shallow.c | 9 +++++++++
> t/t5538-push-shallow.sh | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/shallow.c b/shallow.c
> index 7e0ee96ead9..dcebc263d70 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -489,6 +489,15 @@ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa)
>
> void clear_shallow_info(struct shallow_info *info)
> {
> + if (info->used_shallow) {
> + for (size_t i = 0; i < info->shallow->nr; i++)
> + free(info->used_shallow[i]);
> + free(info->used_shallow);
> + }
> +
> + free(info->need_reachability_test);
> + free(info->reachable);
> + free(info->shallow_ref);
> free(info->ours);
> free(info->theirs);
> }Recently was agreed in the CodingGuidelines `S_clear()` functions do a `S_release()` + `S_init()`. I see we're not initializing the struct for future use (i.e. we don't reset the `nr_*` fields to 0). But we cannot really do an init, because that would be calling `prepare_shallow_info()`, which allocates new memory. So would it be worth to rename this function to `release_shallow_info()`?
-- Toon