Re: [PATCH v3 19/23] refs-be-files.c: add a backend method structure with transaction functions

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2014, 21:38 UTC
Message-ID: <xmqq8umbgcd9.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1408465847-30384-20-git-send-email-sahlberg@google.com>

Ronnie Sahlberg <sahlberg@google.com> writes:

Show 16 quoted lines

> diff --git a/refs-be-files.c b/refs-be-files.c
> index e58a7e1..27eafd0 100644
> --- a/refs-be-files.c
> +++ b/refs-be-files.c
> ...
> +struct ref_be refs_files = {
> +	files_transaction_begin,
> +	files_transaction_update_sha1,
> +	files_transaction_create_sha1,
> +	files_transaction_delete_sha1,
> +	files_transaction_update_reflog,
> +	files_transaction_commit,
> +	files_transaction_free,
> +};
> +
> +struct ref_be *refs = &refs_files;

Show 27 quoted lines

> diff --git a/refs.c b/refs.c
> index 6b434ad..b8c942f 100644
> --- a/refs.c
> +++ b/refs.c
> ...
> +void transaction_free(struct ref_transaction *transaction)
> +{
> +	return refs->transaction_free(transaction);
> +}
> diff --git a/refs.h b/refs.h
> index a14fc5d..4b669f5 100644
> --- a/refs.h
> +++ b/refs.h
> ...
> +struct ref_be {
> +	transaction_begin_fn transaction_begin;
> +	transaction_update_sha1_fn transaction_update_sha1;
> +	transaction_create_sha1_fn transaction_create_sha1;
> +	transaction_delete_sha1_fn transaction_delete_sha1;
> +	transaction_update_reflog_fn transaction_update_reflog;
> +	transaction_commit_fn transaction_commit;
> +	transaction_free_fn transaction_free;
> +};
> +
> +extern struct ref_be *refs;
> +
>  #endif /* REFS_H */

The overall structure is certainly nice, but this means you only can LINK with one backend. Is that what we really want?

I would have expected something like this:

  * In refs.c, there is a "static struct ref_be *the_refs_backend"
    that points at the chosen singleton backend;

  * Upon start-up, set_refs_backend() function that is exported from
    refs.c can be used to set the_refs_backend;

  * Each refs-be-frotz.c will export "struct ref_be refs_frotz" (or
    perhaps "struct refs_be refs_be_frotz") to the outside world, so
    that the start-up code can call set_refs_backend() with it.

  * It is probably sensible to keep the_refs_backend default to
    &refs_be_files.