On Tue, Jul 30, 2024 at 03:06:37PM -0700, Junio C Hamano wrote:
> shejialuo <shejialuo@xxxxxxxxx> 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/fe0e2c3617c8040c632dbc3de613a1d22e8070f7.1722316795.git.ps@xxxxxx/
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.
