Re: [PATCH v2 1/2] promisor-remote: accept 0 as oid_nr in function

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 2, 2020, 19:46 UTC
Message-ID: <xmqqblo93c31.fsf@gitster.c.googlers.com>
In-Reply-To: <474eb27d9c136fb69e961546004cfb531d722e2c.1585854639.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 5 quoted lines

> There are 3 callers to promisor_remote_get_direct() that first check if
> the number of objects to be fetched is equal to 0. Fold that check into
> promisor_remote_get_direct(), and in doing so, be explicit as to what
> promisor_remote_get_direct() does if oid_nr is 0 (it returns 0, success,
> immediately).

Show 9 quoted lines

>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  builtin/index-pack.c |  5 ++---
>  diff.c               | 11 +++++------
>  promisor-remote.c    |  3 +++
>  promisor-remote.h    |  8 ++++++++
>  unpack-trees.c       |  5 ++---
>  5 files changed, 20 insertions(+), 12 deletions(-)

Nice simplification.

> +/*
> + * Fetches all requested objects from all promisor remotes, trying them one at
> + * a time until all objects are fetched. Returns 0 upon success, and non-zero
> + * otherwise.

Good.

> + * If oid_nr is 0, this function returns 0 (success) immediately.

Is this worth saying? If you ask to lazily grab 0 objects, it is probably clear that no object would be read before the helper returns.

When oid_nr==0 you are allowed to pass oids==NULL, but otherwise, oids==NULL would be an error. Is that the kind of difference you wanted to point out, I wonder?

> + */
>  int promisor_remote_get_direct(struct repository *repo,
>  			       const struct object_id *oids,
>  			       int oid_nr);