On 09/29/2015 12:02 AM, David Turner wrote: > The file backend doesn't need this function, but other backends might. > > Signed-off-by: David Turner <dtur...@twopensource.com> > Signed-off-by: Ronnie Sahlberg <rsahlb...@google.com> > --- > refs-be-files.c | 1 + > refs.c | 4 +++- > refs.h | 4 +++- > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/refs-be-files.c b/refs-be-files.c > index 37e244a..eaa74b6 100644 > --- a/refs-be-files.c > +++ b/refs-be-files.c > @@ -3737,6 +3737,7 @@ int reflog_expire(const char *refname, const unsigned > char *sha1, > struct ref_be refs_be_files = { > NULL, > "files", > + NULL, > files_transaction_begin, > files_transaction_update, > files_transaction_create, > diff --git a/refs.c b/refs.c > index 769574d..9ce10b7 100644 > --- a/refs.c > +++ b/refs.c > @@ -19,13 +19,15 @@ struct ref_be *refs_backends = &refs_be_files; > /* > * This function is used to switch to an alternate backend. > */ > -int set_refs_backend(const char *name) > +int set_refs_backend(const char *name, void *init_data) > { > struct ref_be *be; > > for (be = refs_backends; be; be = be->next) > if (!strcmp(be->name, name)) { > the_refs_backend = be; > + if (be->init_backend) > + be->init_backend(init_data);
I don't like that this virtual function, alone among all of them introduced so far, is allowed to be NULL. That seems non-obvious and something extra that devs have to remember. I think it would be better for the files backend to define a do-nothing function that can be stuck in this slot. If you are opposed to that for some reason, then please at least add a comment where this function pointer is added to `struct ref_be`, warning that it can be NULL. > return 0; > } > return 1; > diff --git a/refs.h b/refs.h > index 0b407b2..0dc626e 100644 > --- a/refs.h > +++ b/refs.h > @@ -586,6 +586,7 @@ extern int reflog_expire(const char *refname, const > unsigned char *sha1, > void *policy_cb_data); > > /* refs backends */ > +typedef void (*ref_backend_init_fn)(void *data); > typedef struct ref_transaction *(*ref_transaction_begin_fn)(struct strbuf > *err); > typedef int (*ref_transaction_update_fn)(struct ref_transaction *transaction, > const char *refname, const unsigned char *new_sha1, > @@ -641,6 +642,7 @@ typedef int (*for_each_reftype_fullpath_fn)(each_ref_fn > fn, char *type, > struct ref_be { > struct ref_be *next; > const char *name; > + ref_backend_init_fn init_backend; > ref_transaction_begin_fn transaction_begin; > ref_transaction_update_fn transaction_update; > ref_transaction_create_fn transaction_create; > @@ -669,6 +671,6 @@ struct ref_be { > > > extern struct ref_be refs_be_files; > -int set_refs_backend(const char *name); > +int set_refs_backend(const char *name, void *init_data); > > #endif /* REFS_H */ > Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html