Michael Haggerty <mhagger@xxxxxxxxxxxx> 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?