Re: [PATCH 4/5] files_ref_store::submodule: use NULL for the main repository

[Junio C Hamano <gitster@pobox.com>]

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