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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?


[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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]