Subject: Re: [GSoC][PATCH v13 10/10] fsck: add ref content check for files backend

From: Junio C Hamano <[email-masked]>
Date: Tue, 30 Jul 2024 15:06:37 -0700

shejialuo <[email-masked]> writes:

> +static int files_fsck_refs_content(struct fsck_options *o,
> +				   const char *gitdir,
> +				   const char *refs_check_dir,
> +				   struct dir_iterator *iter)
> +{
> + ...
> +	if (parse_loose_ref_contents(ref_content.buf, &oid,
> +				     &referent, &type,
> +				     &failure_errno, &trailing)) {

The function parse_loose_ref_contents() needs to know what the hash
algorithm is, and it used to implicitly assume that the_repository's
hash algorithm was an OK thing to use.  Patrick's recent clean-up
series instead passes "struct ref_store *refs" throughout the call
chain so that "ref->repo->hash_algo"  can be used.  This needs some
matching change, which means ...

>  	files_fsck_refs_fn fsck_refs_fns[]= {
>  		files_fsck_refs_name,
> +		files_fsck_refs_content,
>  		NULL
>  	};

... the function signature for files_fsck_refs_fn must change to
have something that lets you access repo->hash_algo.

By the way, unless the most common use of an array is to pass it
around as a collection of items and operate on the collection, it is
a better practice to name an array with a singular noun.  Name the
array as fsck_refs_fn[] not _fns[].  This is so that you can refer
to a single element in a more grammatical way.  E.g. with

  struct dog dog[] = { { .breed="shiba" }, { .breed="beagle" } };

you can say "dog[0] has brown fur" instead of "dogs[0] has ...".

In this case, you do not treat the collection of functions as a one
thing and do something to the collection.  Instead you'd repeat over
the functions in a loop and individually call them, perhaps like so:

	for (i = 0; fsck_fn[i] != NULL; i++)
		fsck_fn[i](...);

so it is very much more appropriate to name the array itself as
singular to allow you to say "first fsck_fn", "next fsck_fn", etc.