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

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > +
> > +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?

> >  /*
> >   * 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");