Michael Haggerty <mhag...@alum.mit.edu> writes:

> The old practice of storing the empty string in this member for the main
> repository was a holdover from before 00eebe3 (refs: create a base class
> "ref_store" for files_ref_store, 2016-09-04), when the submodule was
> stored in a flex array at the end of `struct files_ref_store`. Storing
> NULL for this case is more idiomatic and a tiny bit less code.

Yes.  I noticed this bit in 3/5 and wondered about it, knowing this
step comes next:

>  struct ref_store *ref_store_init(const char *submodule)
>  {
>       const char *be_name = "files";
>       struct ref_storage_be *be = find_ref_storage_backend(be_name);
> +     struct ref_store *refs;
>  
>       if (!be)
>               die("BUG: reference backend %s is unknown", be_name);
>  
>       if (!submodule || !*submodule)
> -             return be->init(NULL);
> +             refs = be->init(NULL);
>       else
> -             return be->init(submodule);
> +             refs = be->init(submodule);

Can't we also lose this "if !*submodule, turn it to NULL"?

> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -980,7 +980,7 @@ static struct ref_store *files_ref_store_create(const 
> char *submodule)
>       struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
>       struct ref_store *ref_store = (struct ref_store *)refs;
>  
> -     base_ref_store_init(ref_store, &refs_be_files, submodule);
> +     base_ref_store_init(ref_store, &refs_be_files);
>  
>       refs->submodule = submodule ? xstrdup(submodule) : "";

Also, can't we use xstrdup_or_null(submodule) with this step?

Reply via email to