Re: [PATCH 02/16] refs: add methods for misc ref operations

Junio C Hamano <gitster <at> pobox.com> writes:

David Turner <dturner <at> twopensource.com> writes:

quoted

 struct ref_be {
 	struct ref_be *next;
 	const char *name;
 	ref_transaction_commit_fn *transaction_commit;
+
+	pack_refs_fn *pack_refs;
+	peel_ref_fn *peel_ref;
+	create_symref_fn *create_symref;
+
+	resolve_ref_unsafe_fn *resolve_ref_unsafe;
+	verify_refname_available_fn *verify_refname_available;
+	resolve_gitlink_ref_fn *resolve_gitlink_ref;
 };

This may have been pointed out in the previous reviews by somebody
else, but I think it is more customary to declare a struct member
that is a pointer to a customization function without leading '*',
i.e.

	typedef TYPE (*customize_fn)(ARGS);

        struct vtable {
		...
        	cutomize_fn fn;
		...
	};

in our codebase (cf. string_list::cmp, prio_queue::compare).

(LMDB author here, just passing by)

IMO you're making a mistake. You should always typedef the thing itself, not
a pointer to the thing. If you only typedef the pointer, you can only use
the typedef to declare pointers and never the thing itself. If you typedef
the actual thing, you can use the typedef to declare both the thing and
pointer to thing.

It's particularly useful to have function typedefs because later on you can
use the actual typedef to declare instances of that function type in header
files, and guarantee that your function definitions match what they're
intended to match. Otherwise, assigning a bare (*func) to something that
mismatches only generates a warning, instead of an error.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/