On Tue, Aug 26, 2014 at 2:25 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ronnie Sahlberg <sahlberg@xxxxxxxxxx> 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?
>
>
Thanks!
I did these changes :
1, rename to the_refs_backend
2, add a function set_refs_backend()
To this series and update it at
https://github.com/rsahlberg/git/tree/backend-struct-db
> [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);
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html