Re: [PATCH 3/5] register_ref_store(): new function

[Stefan Beller <sbeller@xxxxxxxxxx>]

On Thu, Feb 9, 2017 at 5:27 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> Move the responsibility for registering the ref_store for a submodule
> from base_ref_store_init() to a new function, register_ref_store(). Call
> the latter from ref_store_init().
>
> This means that base_ref_store_init() can lose its submodule argument,
> further weakening the 1:1 relationship between ref_stores and
> submodules.
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---




> +
>  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);
> +
> +       register_ref_store(refs, submodule);
> +       return refs;
>  }

This function is already very readable, though maybe it would be
more readable like so:

{
    const char *be_name = "files";
    struct ref_storage_be *be = find_ref_storage_backend(be_name);

    if (!be)
        die("BUG: reference backend %s is unknown", be_name);

    /* replace empty string by NULL */
    if (submodule && !*submodule)
        submodule = NULL;

    register_ref_store(be->init(submodule), submodule);
    return refs;
}

Well, I dunno; the function inside the arguments to register seems ugly, though.