Re: [PATCH] promisor-remote.c: use oidset for deduplication

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 13, 2022, 23:45 UTC
Message-ID: <xmqqilunfber.fsf@gitster.g>
In-Reply-To: <pull.1187.git.git.1642105926064.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 22 quoted lines

> diff --git a/object-file.c b/object-file.c
> index 8be57f48de7..baec8f099de 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1519,6 +1519,7 @@ static int do_oid_object_info_extended(struct repository *r,
>  	struct cached_object *co;
>  	struct pack_entry e;
>  	int rtype;
> +	struct oidset to_fetch = OIDSET_INIT;
>  	const struct object_id *real = oid;
>  	int already_retried = 0;
>  
> @@ -1587,7 +1588,8 @@ static int do_oid_object_info_extended(struct repository *r,
>  			 * TODO Investigate checking promisor_remote_get_direct()
>  			 * TODO return value and stopping on error here.
>  			 */
> -			promisor_remote_get_direct(r, real, 1);
> +			oidset_insert(&to_fetch, real);
> +			promisor_remote_get_direct(r, &to_fetch);
>  			already_retried = 1;
>  			continue;
>  		}

Everything that migrated from oid_array to oidset should be OK because it is likely that existing oid_array_clear() would not compile until it is replaced with oidset_clear(), at which point the oidset will be cleared when we are done with it.

However, this one did not use an oid_array and did not have to clear to release the resources. So it is very likely that nobody clears to_fetch that is introduced by this patch, no?

I suspect that this introduces a new leak.