Re: [PATCH 3/3] bundle: remove "ref_list" in favor of string-list.c API

From: Andrei Rybak <rybak.a.v@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 3/3] bundle: remove "ref_list" in favor of string-list.c API
Date: Sat, 19 Jun 2021 04:12:45 +0200	[thread overview]
Message-ID: <23c4ce5f-7769-1d2c-3a97-ac9733f73a25@gmail.com> (raw)
In-Reply-To: <patch-3.3-887313d3b0-20210617T111934Z-avarab@gmail.com>

On 17/06/2021 13:21, Ævar Arnfjörð Bjarmason wrote:
> ---
>   builtin/bundle.c | 12 ++++-----
>   bundle.c         | 64 ++++++++++++++++++++++++------------------------
>   bundle.h         | 20 +++++++--------
>   transport.c      | 10 +++++---
>   4 files changed, 55 insertions(+), 51 deletions(-)
> 

[snip]

> diff --git a/bundle.c b/bundle.c
> index 621708f40e..d36eeee1a5 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -162,14 +156,14 @@ static int list_refs(struct ref_list *r, int argc, const char **argv)
>   		if (argc > 1) {
>   			int j;
>   			for (j = 1; j < argc; j++)
> -				if (!strcmp(r->list[i].name, argv[j]))
> +				if (!strcmp(r->items[i].string, argv[j]))
>   					break;
>   			if (j == argc)
>   				continue;
>   		}
>   
> -		oid = &r->list[i].oid;
> -		name = r->list[i].name;
> +		oid = r->items[i].util;
> +		name = r->items[i].string;
>   		printf("%s %s\n", oid_to_hex(oid), name);

In function `list_refs` variable `name` that is used in a call to printf
has been extracted by the previous patch.

>   	}
>   	return 0;
> @@ -186,7 +180,7 @@ int verify_bundle(struct repository *r,
>   	 * Do fast check, then if any prereqs are missing then go line by line
>   	 * to be verbose about the errors
>   	 */
> -	struct ref_list *p = &header->prerequisites;
> +	struct string_list *p = &header->prerequisites;
>   	struct rev_info revs;
>   	const char *argv[] = {NULL, "--all", NULL};
>   	struct commit *commit;
> @@ -198,17 +192,17 @@ int verify_bundle(struct repository *r,
>   
>   	repo_init_revisions(r, &revs, NULL);
>   	for (i = 0; i < p->nr; i++) {
> -		struct ref_list_entry *e = p->list + i;
> -		struct object_id *oid = &e->oid;
> +		struct string_list_item *e = p->items + i;
> +		struct object_id *oid = e->util;
>   		struct object *o = parse_object(r, oid);
>   		if (o) {
>   			o->flags |= PREREQ_MARK;
> -			add_pending_object(&revs, o, e->name);
> +			add_pending_object(&revs, o, e->string);
>   			continue;
>   		}
>   		if (++ret == 1)
>   			error("%s", message);
> -		error("%s %s", oid_to_hex(oid), e->name);
> +		error("%s %s", oid_to_hex(oid), e->string);
>   	}
>   	if (revs.pending.nr != p->nr)
>   		return ret;
> @@ -224,28 +218,28 @@ int verify_bundle(struct repository *r,
>   			i--;
>   
>   	for (i = 0; i < p->nr; i++) {
> -		struct ref_list_entry *e = p->list + i;
> -		struct object_id *oid = &e->oid;
> +		struct string_list_item *e = p->items + i;
> +		const struct object_id *oid = e->util;
>   		struct object *o = parse_object(r, oid);
>   		assert(o); /* otherwise we'd have returned early */
>   		if (o->flags & SHOWN)
>   			continue;
>   		if (++ret == 1)
>   			error("%s", message);
> -		error("%s %s", oid_to_hex(oid), e->name);
> +		error("%s %s", oid_to_hex(oid), e->string);

However, `e->name` in three places in function `verify_bundle` for two
different instances of `struct ref_list_entry *` wasn't extracted into
a variable by the previous patch. Could you please clarify this
discrepancy?

[snip]

> diff --git a/transport.c b/transport.c
> index 9d601c8c95..667c4e0cc6 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -147,13 +147,14 @@ static struct ref *get_refs_from_bundle(struct transport *transport,
>   	transport->hash_algo = data->header.hash_algo;
>   
>   	for (i = 0; i < data->header.references.nr; i++) {
> -		struct ref_list_entry *e = data->header.references.list + i;
> -		struct ref *ref = alloc_ref(e->name);
> -		struct object_id *oid = &e->oid;
> +		struct string_list_item *e = data->header.references.items + i;
> +		struct ref *ref = alloc_ref(e->string);