Re: [PATCH v2 18/23] refs.c: add a backend method structure with transaction functions

> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> +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;
>
> The overall organization is nice, but please don't use such a short
> name for the systemwide default singleton instance, which should not
> be accessed by normal code other than via helpers that implicitly
> use that singleton (e.g. resolve_ref_unsafe() which invokes the
> method of the same name on the singleton, passing the parameters it
> received[*1*]).  The name will be used for other things (e.g. a
> local variable for a collection of refs) by code that do not care
> about the underlying implementation of the helpers and will cause
> confusion later.
>
> Perhaps the_refs_backend or something?
>
> Also does the singleton have to be extern, not a static inside refs.c,
> perhaps with a setter function to switch it or something?
>
>

> [Reference]
>
> *1* A typical helper that uses the singleton looks like this:
>
> +const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
> +                              int reading, int *flag)
> +{
> +       return refs->resolve_ref_unsafe(ref, sha1, reading, flag);
> +}