Michael Haggerty <mhagger@xxxxxxxxxxxx> writes: > Build out the API for dealing with a bunch of reference checks and > changes within a transaction. Define an opaque ref_transaction type > that is managed entirely within refs.c. Introduce functions for > beginning a transaction, adding updates to a transaction, and > committing/rolling back a transaction. > > This API will soon replace update_refs(). > > Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx> > --- > refs.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > refs.h | 65 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 161 insertions(+) > > diff --git a/refs.c b/refs.c > index 1305eb1..e788c27 100644 > --- a/refs.c > +++ b/refs.c > @@ -3267,6 +3267,93 @@ static int update_ref_write(const char *action, const char *refname, > return 0; > } > > +/* > + * Data structure for holding a reference transaction, which can > + * consist of checks and updates to multiple references, carried out > + * as atomically as possible. This structure is opaque to callers. > + */ > +struct ref_transaction { > + struct ref_update **updates; Don't we try to name an array update[] (not plural updates[]) so that we can say update[7] to mean the seventh update? > + size_t alloc; > + size_t nr; > +}; > + > +struct ref_transaction *ref_transaction_begin(void) > +{ > + return xcalloc(1, sizeof(struct ref_transaction)); > +} > + > +static void ref_transaction_free(struct ref_transaction *transaction) > +{ > + int i; > + > + for (i = 0; i < transaction->nr; i++) { > + struct ref_update *update = transaction->updates[i]; > + > + free((char *)update->ref_name); > + free(update); > + } > + > + free(transaction->updates); > + free(transaction); > +} OK. > +void ref_transaction_rollback(struct ref_transaction *transaction) > +{ > + ref_transaction_free(transaction); > +} OK. > +static struct ref_update *add_update(struct ref_transaction *transaction, > + const char *refname) > +{ > + struct ref_update *update = xcalloc(1, sizeof(*update)); > + > + update->ref_name = xstrdup(refname); > + ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc); > + transaction->updates[transaction->nr++] = update; > + return update; > +} > + > +void ref_transaction_update(struct ref_transaction *transaction, > + const char *refname, > + unsigned char *new_sha1, unsigned char *old_sha1, > + int flags, int have_old) > +{ > + struct ref_update *update = add_update(transaction, refname); > + > + hashcpy(update->new_sha1, new_sha1); > + update->flags = flags; > + update->have_old = have_old; > + if (have_old) > + hashcpy(update->old_sha1, old_sha1); > +} > + > +void ref_transaction_create(struct ref_transaction *transaction, > + const char *refname, > + unsigned char *new_sha1, > + int flags) > +{ > + struct ref_update *update = add_update(transaction, refname); > + > + hashcpy(update->new_sha1, new_sha1); > + hashclr(update->old_sha1); > + update->flags = flags; > + update->have_old = 1; > +} > + > +void ref_transaction_delete(struct ref_transaction *transaction, > + const char *refname, > + unsigned char *old_sha1, > + int flags, int have_old) > +{ > + struct ref_update *update = add_update(transaction, refname); > + > + update->flags = flags; > + update->have_old = have_old; > + if (have_old) > + hashcpy(update->old_sha1, old_sha1); > +} > + I can see that the chosen set of primitives update/create/delete mirrors what update-ref allows us to do, but given the explanation of "update" in refs.h, wouldn't it make more sense to implement the others in terms of it? > int update_ref(const char *action, const char *refname, > const unsigned char *sha1, const unsigned char *oldval, > int flags, enum action_on_err onerr) > @@ -3378,6 +3465,15 @@ cleanup: > return ret; > } > > +int ref_transaction_commit(struct ref_transaction *transaction, > + const char *msg, enum action_on_err onerr) > +{ > + int ret = update_refs(msg, transaction->updates, transaction->nr, > + onerr); > + ref_transaction_free(transaction); > + return ret; > +} OK. > char *shorten_unambiguous_ref(const char *refname, int strict) > { > int i; > diff --git a/refs.h b/refs.h > index 08e60ac..476a923 100644 > --- a/refs.h > +++ b/refs.h > @@ -24,6 +24,8 @@ struct ref_update { > int have_old; /* 1 if old_sha1 is valid, 0 otherwise */ > }; > > +struct ref_transaction; > + > /* > * Bit values set in the flags argument passed to each_ref_fn(): > */ > @@ -220,6 +222,69 @@ enum action_on_err { > UPDATE_REFS_QUIET_ON_ERR > }; > > +/* > + * Begin a reference transaction. The reference transaction must > + * eventually be commited using ref_transaction_commit() or rolled > + * back using ref_transaction_rollback(). > + */ > +struct ref_transaction *ref_transaction_begin(void); > + > +/* > + * Roll back a ref_transaction and free all associated data. > + */ > +void ref_transaction_rollback(struct ref_transaction *transaction); > + > + > +/* > + * The following functions add a reference check or update to a > + * ref_transaction. In all of them, refname is the name of the > + * reference to be affected. The functions make internal copies of > + * refname, so the caller retains ownership of the parameter. flags Good to see the ownership rules described. > + * can be REF_NODEREF; it is passed to update_ref_lock(). > + */ > + > + > +/* > + * Add a reference update to transaction. new_sha1 is the value that > + * the reference should have after the update, or zeros if it should > + * be deleted. If have_old is true, then old_sha1 holds the value > + * that the reference should have had before the update, or zeros if > + * it must not have existed beforehand. > + */ > +void ref_transaction_update(struct ref_transaction *transaction, > + const char *refname, > + unsigned char *new_sha1, unsigned char *old_sha1, > + int flags, int have_old); > + > +/* > + * Add a reference creation to transaction. new_sha1 is the value > + * that the reference should have after the update, or zeros if it > + * should be deleted. It is verified that the reference does not > + * exist already. > + */ Sounds a bit crazy that you can ask "create", which verifies the absense of the thing, to delete a thing. > +void ref_transaction_create(struct ref_transaction *transaction, > + const char *refname, > + unsigned char *new_sha1, > + int flags); > + > +/* > + * Add a reference deletion to transaction. If have_old is true, then > + * old_sha1 holds the value that the reference should have had before > + * the update. > + */ > +void ref_transaction_delete(struct ref_transaction *transaction, > + const char *refname, > + unsigned char *old_sha1, > + int flags, int have_old); > + > +/* > + * Commit all of the changes that have been queued in transaction, as > + * atomically as possible. Return a nonzero value if there is a > + * problem. The ref_transaction is freed by this function. > + */ > +int ref_transaction_commit(struct ref_transaction *transaction, > + const char *msg, enum action_on_err onerr); > + > /** Lock a ref and then write its file */ > int update_ref(const char *action, const char *refname, > const unsigned char *sha1, const unsigned char *oldval, -- 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