Re: [PATCH 03/12] refs: refactor logic to look up storage backends

From: Karthik Nayak <karthik.188@gmail.com>
Date: Dec 22, 2023, 12:38 UTC
Message-ID: <CAOLa=ZR1Bc+Acwjma9OM0SvyLg4-HB-S3Pxr66VCKTb0d_tdnw@mail.gmail.com>
In-Reply-To: <f712d5ef5bc5bed423cf50a0d0489095ba103df7.1703067989.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 9 quoted lines

> +
> +const char *ref_storage_format_to_name(int ref_storage_format)
> +{
> +	const struct ref_storage_be *be = find_ref_storage_backend(ref_storage_format);
> +	if (!be)
> +		return "unknown";
> +	return be->name;
> +}
>

Would it make more sense to return NULL here?

Show 18 quoted lines

> +
>  /*
>   * How to handle various characters in refnames:
>   * 0: An acceptable character for refs
> @@ -2029,12 +2045,12 @@ static struct ref_store *ref_store_init(struct repository *repo,
>  					const char *gitdir,
>  					unsigned int flags)
>  {
> -	const char *be_name = "files";
> -	struct ref_storage_be *be = find_ref_storage_backend(be_name);
> +	int format = REF_STORAGE_FORMAT_FILES;
> +	const struct ref_storage_be *be = find_ref_storage_backend(format);
>  	struct ref_store *refs;
>
>  	if (!be)
> -		BUG("reference backend %s is unknown", be_name);
> +		BUG("reference backend %s is unknown", ref_storage_format_to_name(format));
>

This doesn't really get us more information, since be == NULL, calling `ref_storage_format_to_name` will again call `find_ref_storage_backend`, which will be NULL. I wonder if its better to:

@@ -2029,12 +2045,12 @@ static struct ref_store *ref_store_init(struct
repository *repo,
                     const char *gitdir,
                     unsigned int flags)
  {
 -	const char *be_name = "files";
 -	struct ref_storage_be *be = find_ref_storage_backend(be_name);
 +	int format = REF_STORAGE_FORMAT_FILES;
 +	const struct ref_storage_be *be = find_ref_storage_backend(format);
     struct ref_store *refs;

     if (!be)
 -		BUG("reference backend %s is unknown", be_name);
 +		BUG("reference backend is unknown");