Re: [PATCH 06/10] packed-backend: add "packed-refs" entry consistency check

[Patrick Steinhardt <ps@pks.im>]

On Sun, Jan 05, 2025 at 09:49:59PM +0800, shejialuo wrote:
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index f6142a4402..6e521a9f87 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -1822,7 +1822,96 @@ static int packed_fsck_ref_header(struct fsck_options *o, const char *start, con
>  	return 0;
>  }
>  
> +static int packed_fsck_ref_peeled_line(struct fsck_options *o,
> +				       struct ref_store *ref_store, int line_number,
> +				       const char *start, const char *eol)
> +{
> +	struct strbuf peeled_entry = STRBUF_INIT;
> +	struct fsck_ref_report report = { 0 };
> +	struct object_id peeled;
> +	const char *p;
> +	int ret = 0;
> +
> +	strbuf_addf(&peeled_entry, "packed-refs line %d", line_number);
> +	report.path = peeled_entry.buf;
> +
> +	start++;
> +	if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) {
> +		ret |= fsck_report_ref(o, &report,
> +				       FSCK_MSG_BAD_PACKED_REF_ENTRY,
> +				       "'%.*s' has invalid peeled oid",
> +				       (int)(eol - start), start);
> +		goto cleanup;
> +	}
> +
> +	if (p != eol) {
> +		ret |= fsck_report_ref(o, &report,
> +				       FSCK_MSG_BAD_PACKED_REF_ENTRY,
> +				       "has trailing garbage after peeled oid '%.*s'",
> +				       (int)(eol - p), p);
> +		goto cleanup;
> +	}
> +
> +cleanup:
> +	strbuf_release(&peeled_entry);
> +	return ret;
> +}
> +
> +static int packed_fsck_ref_main_line(struct fsck_options *o,
> +				     struct ref_store *ref_store, int line_number,
> +				     const char *start, const char *eol)
> +{
> +	struct strbuf packed_entry = STRBUF_INIT;
> +	struct fsck_ref_report report = { 0 };
> +	struct strbuf refname = STRBUF_INIT;

It feels quite inefficient to create a separate buffer for every
invocation of this function, as there can be many million refs in a
repo. Might be something to avoid by passing in a scratch buffer.

Patrick