On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> This is the last function in this code (besides public API) that takes
> submodule argument and handles both main/submodule cases. Break it down,
> move main store registration in get_main_ref_store() and keep the rest
> in register_submodule_ref_store().
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
> ---
> refs.c | 50 ++++++++++++++++++++++++++------------------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index c284cb4f4..6adc38e42 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1412,29 +1412,6 @@ static struct ref_store
> *lookup_submodule_ref_store(const char *submodule)
> }
>
> /*
> - * Register the specified ref_store to be the one that should be used
> - * for submodule (or the main repository if submodule is NULL). It is
> - * a fatal error to call this function twice for the same submodule.
> - */
> -static void register_ref_store(struct ref_store *refs, const char *submodule)
> -{
> - if (!submodule) {
> - if (main_ref_store)
> - die("BUG: main_ref_store initialized twice");
> -
> - main_ref_store = refs;
> - } else {
> - if (!submodule_ref_stores.tablesize)
> - hashmap_init(&submodule_ref_stores, submodule_hash_cmp,
> 0);
> -
> - if (hashmap_put(&submodule_ref_stores,
> - alloc_submodule_hash_entry(submodule, refs)))
> - die("BUG: ref_store for submodule '%s' initialized
> twice",
> - submodule);
> - }
> -}
> -
> -/*
> * Create, record, and return a ref_store instance for the specified
> * submodule (or the main repository if submodule is NULL).
> */
> @@ -1448,7 +1425,6 @@ static struct ref_store *ref_store_init(const char
> *submodule)
> die("BUG: reference backend %s is unknown", be_name);
>
> refs = be->init(submodule);
> - register_ref_store(refs, submodule);
> return refs;
> }
>
> @@ -1460,9 +1436,32 @@ static struct ref_store *get_main_ref_store(void)
> return main_ref_store;
>
> refs = ref_store_init(NULL);
> + if (refs) {
> + if (main_ref_store)
> + die("BUG: main_ref_store initialized twice");
> +
> + main_ref_store = refs;
> + }
> return refs;
Superfluous test: I don't think `ref_store_init()` ever returns NULL.
This also implies that you could check `main_ref_store` before calling
`ref_store_init()`, and eliminate a temporary.
> }
>
> +/*
> + * Register the specified ref_store to be the one that should be used
> + * for submodule. It is a fatal error to call this function twice for
> + * the same submodule.
> + */
> +static void register_submodule_ref_store(struct ref_store *refs,
> + const char *submodule)
> +{
> + if (!submodule_ref_stores.tablesize)
> + hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);
> +
> + if (hashmap_put(&submodule_ref_stores,
> + alloc_submodule_hash_entry(submodule, refs)))
> + die("BUG: ref_store for submodule '%s' initialized twice",
> + submodule);
> +}
> +
> struct ref_store *get_ref_store(const char *submodule)
> {
> struct strbuf submodule_sb = STRBUF_INIT;
> @@ -1480,6 +1479,9 @@ struct ref_store *get_ref_store(const char *submodule)
> if (is_nonbare_repository_dir(&submodule_sb))
> refs = ref_store_init(submodule);
> strbuf_release(&submodule_sb);
> +
> + if (refs)
I think `refs` should always be non-NULL here for the same reason.
> + register_submodule_ref_store(refs, submodule);
> return refs;
> }
Michael
