Subject: Re: [GSoC][PATCH v10 03/10] fsck: add a unified interface for reporting fsck messages

From: Junio C Hamano <[email-masked]>
Date: Wed, 10 Jul 2024 14:04:15 -0700

shejialuo <[email-masked]> writes:

> The static function "report" provided by "fsck.c" aims at checking fsck
> error type and calling the callback "error_func" to report the message.
> However, "report" function is only related to object database which
> cannot be reused for refs. In order to provide a unified interface which
> can report either objects or refs, create a new function "vfsck_report"
> by adding "checked_ref_name" parameter following the "report" prototype.
> Instead of using "...", provide "va_list" to allow more flexibility.

Like strbuf_vinsertf(), it is a good idea to have "v" in the name of
a function that takes va_list, but fsck_vreport() would probably be
a better name here.  Arguably, the original report() is misnamed (as
a printf-like function that takes format string, it probably would
have wanted to be reportf() instead), but unless we are fixing that
at the same time, calling this fsck_vreportf() would probably be too
much.  Consistently misnaming it by omitting the final "f" would be
fine.

At this step it is still not clear if the previous step was really
needed; you have this "v" thing that is designed to be usable by
both reporting issues around objects and issues around refs, but we
will hopefully see why when we read later patches.
> diff --git a/object-file.c b/object-file.c
> index 065103be3e..d2c6427935 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2470,11 +2470,12 @@ int repo_has_object_file(struct repository *r,
>   * give more context.
>   */
>  static int hash_format_check_report(struct fsck_options *opts UNUSED,
> -				     const struct object_id *oid UNUSED,
> -				     enum object_type object_type UNUSED,
> -				     enum fsck_msg_type msg_type UNUSED,
> -				     enum fsck_msg_id msg_id UNUSED,
> -				     const char *message)
> +				    const struct object_id *oid UNUSED,
> +				    enum object_type object_type UNUSED,
> +				    const char *ref_checked_name UNUSED,
> +				    enum fsck_msg_type msg_type UNUSED,
> +				    enum fsck_msg_id msg_id UNUSED,
> +				    const char *message)

That is somewhat annoying reindentation.  What happened here?