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

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

 



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

> 
> David Turner <dturner <at> twopensource.com> writes:
> 
> >  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/ 


--
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]