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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 30, 2024, 22:06 UTC
Message-ID: <xmqqh6c6a61e.fsf@gitster.g>
In-Reply-To: <ZqeY3Dhj-Fo-bZ2k@ArchLinux>

shejialuo <shejialuo@gmail.com> writes:

Show 9 quoted lines

> +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 ...

Show 5 quoted lines

>  	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.