Re: [PATCH v4 19/21] refs: add register_ref_storage_backends()

[Michael Haggerty <mhagger@alum.mit.edu>]

On 02/05/2016 08:44 PM, David Turner wrote:
> This new function will register all known ref storage backends... once
> there are any other than the default.  For now, it's a no-op.
> 
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  builtin/init-db.c |  3 +++
>  config.c          | 25 +++++++++++++++++++++++++
>  refs.c            |  8 ++++++++
>  refs.h            |  2 ++
>  4 files changed, 38 insertions(+)
> 
> [...]
> diff --git a/config.c b/config.c
> index b95ac3a..b9ef223 100644
> --- a/config.c
> +++ b/config.c
> @@ -11,6 +11,7 @@
>  #include "strbuf.h"
>  #include "quote.h"
>  #include "hashmap.h"
> +#include "refs.h"
>  #include "string-list.h"
>  #include "utf8.h"
>  
> @@ -1207,6 +1208,30 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  	}
>  
>  	if (repo_config && !access_or_die(repo_config, R_OK, 0)) {
> +		char *storage = NULL;
> +
> +		/*
> +		 * make sure we always read the ref storage config
> +		 * from the extensions section on startup
> +		 */
> +		ret += git_config_from_file(ref_storage_backend_config,
> +					    repo_config, &storage);
> +
> +		register_ref_storage_backends();
> +		if (!storage)
> +			storage = xstrdup("");
> +
> +		if ((!*storage) ||
> +		    (!strcmp(storage, "files"))) {

Nit: you have some unnecessary parentheses here.

> +			/* default backend, nothing to do */
> +			free(storage);
> +		} else {
> +			ref_storage_backend = storage;
> +			if (set_ref_storage_backend(ref_storage_backend))
> +				die(_("Unknown ref storage backend %s"),
> +				    ref_storage_backend);
> +		}
> +
>  		ret += git_config_from_file(fn, repo_config, data);
>  		found += 1;
>  	}
> diff --git a/refs.c b/refs.c
> index 715a492..e50cca0 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1554,3 +1554,11 @@ done:
>  	string_list_clear(&affected_refnames, 0);
>  	return ret;
>  }
> +
> +void register_ref_storage_backends(void) {
> +	/*
> +	 * No need to register the files backend; it's registered by
> +	 * default. Add register_ref_storage_backend(ptr-to-backend)
> +	 * entries below when you add a new backend.
> +	 */

This function must be called every run, right? So why not make it
register the "files" backend explicitly? That would make it obvious
really quick if this function fails to get called in some code path.
Just a thought...

> +}
> [...]

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu