On Fri, Dec 22, 2023 at 04:38:30AM -0800, Karthik Nayak wrote:
> Patrick Steinhardt <ps@xxxxxx> 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?
The only purpose of this function will be to print error messages, so
"unknown" seems like the better choice.
> > /*
> > * 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");
Good point, will change.
Patrick
Attachment:
signature.asc
Description: PGP signature
