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

From: shejialuo <[email-masked]>
Date: Thu, 1 Aug 2024 00:19:13 +0800

On Tue, Jul 30, 2024 at 03:06:37PM -0700, Junio C Hamano wrote:
> 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.
> 

Thanks for your remind, I have scanned this patch:

	https://lore.kernel.org/git/[email-masked]/

I guess I will handle this later. It seems that this series has not come
into the cooking tree. I will update this part until Patrick's patch
gets merged into "next".

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

Thanks, I have learned a lot here. I will improve this in the next
version.