From: shejialuo <shejialuo@gmail.com>
Date: Sat, 10 Jan 2026 13:31:07 GMT
Subject: Re: [PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`
Message-ID: <aWJUm-hrPquegbdf@ArchLinux>
In-Reply-To: <20260109-pks-refs-verify-fixes-v1-16-3587dba18294@pks.im>

On Fri, Jan 09, 2026 at 01:39:45PM +0100, Patrick Steinhardt wrote:
> Move the check that detects "HEAD" refs that do not point at a branch
> into `refs_fsck()`. This follows the same motivation as the preceding
> commit.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/fsck-msgids.adoc |  3 +++
>  builtin/fsck.c                 |  7 -------
>  fsck.h                         |  1 +
>  refs.c                         | 12 +++++++++++-
>  t/t0602-reffiles-fsck.sh       |  8 ++++----
>  t/t1450-fsck.sh                |  4 ++--
>  6 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc
> index 76609321f6..6a4db3a991 100644
> --- a/Documentation/fsck-msgids.adoc
> +++ b/Documentation/fsck-msgids.adoc
> @@ -13,6 +13,9 @@
>  `badGpgsig`::
>  	(ERROR) A tag contains a bad (truncated) signature (e.g., `gpgsig`) header.
>  
> +`badHeadTarget`::
> +	(ERROR) The `HEAD` ref is a symref that does not refer to a branch.
> +
>  `badHeaderContinuation`::
>  	(ERROR) A continuation header (such as for `gpgsig`) is unexpectedly truncated.
>  
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 4dd4d74d1e..5dda441f45 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -728,13 +728,6 @@ static void fsck_head_link(const char *head_ref_name,
>  		error(_("invalid %s"), head_ref_name);
>  		return;
>  	}
> -	if (strcmp(*head_points_at, head_ref_name) &&
> -	    !starts_with(*head_points_at, "refs/heads/")) {
> -		errors_found |= ERROR_REFS;
> -		error(_("%s points to something strange (%s)"),
> -		      head_ref_name, *head_points_at);
> -		return;
> -	}
>  
>  	return;
>  }
> diff --git a/fsck.h b/fsck.h
> index 1f472b7daa..65ecbb7fe1 100644
> --- a/fsck.h
> +++ b/fsck.h
> @@ -30,6 +30,7 @@ enum fsck_msg_type {
>  	FUNC(BAD_DATE_OVERFLOW, ERROR) \
>  	FUNC(BAD_EMAIL, ERROR) \
>  	FUNC(BAD_GPGSIG, ERROR) \
> +	FUNC(BAD_HEAD_TARGET, ERROR) \
>  	FUNC(BAD_NAME, ERROR) \
>  	FUNC(BAD_OBJECT_SHA1, ERROR) \
>  	FUNC(BAD_PACKED_REF_ENTRY, ERROR) \
> diff --git a/refs.c b/refs.c
> index c3528862c6..a772d371cd 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,
>  
>  int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,
>  		     struct fsck_ref_report *report,
> -		     const char *refname UNUSED, const char *target)
> +		     const char *refname, const char *target)
>  {
> +	const char *stripped_refname;
> +
> +	parse_worktree_ref(refname, NULL, NULL, &stripped_refname);
> +
> +	if (!strcmp(stripped_refname, "HEAD") &&
> +	    !starts_with(target, "refs/heads/") &&

We would first check whether the current ref is `HEAD`. And I am
wondering whether we have some common APIs. And I find the similar logic
in `reglog.c::is_head` like the following shows:

    static int is_head(const char *refname)
    {
            const char *stripped_refname;
            parse_worktree_ref(refname, NULL, NULL, &stripped_refname);
            return !strcmp(stripped_refname, "HEAD");
    }

I think we might just extract this common logic to avoid introducing
repetition.

Thanks,
Jialuo