Re: [PATCH 4/5] files_ref_store::submodule: use NULL for the main repository
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 9, 2017, 21:19 UTC
- Message-ID
- <xmqq1sv7oyei.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <111d663c0fd3e9669e7c28537f581833488ca4a6.1486629195.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 5 quoted lines
> 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:
Show 15 quoted lines
> 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"?
Show 10 quoted lines
> --- 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?