Below is the delta needed to fixed the bug in the mh/split-under-lock
patch series that I mentioned in an earlier email [1], plus a little
tweak to make the docstring for lock_ref_for_update() clearer.
I actually fixed the bug in preparatory commit
ref_transaction_commit(): remove local variable n
, whose subject was accordingly changed to
ref_transaction_commit(): remove local variables n and updates
[2]. See branch "split-under-lock" in my GitHub fork [3] for the full
revised patch series (including the changes to a later patch that are
necessary when it is rebased onto the fix). In the same repo I've also
rebased dependent branches update-ref-errors, ref-iterators, and
ref-store on top of this one; all of those rebases were trivial.
Here, for the convenience of reviewers, I present the delta between
the old and new end states of the split-under-lock patch series. (It
didn't seem justified to re-send the whole 33-patch series for this
logically small change.)
Michael
[1] http://article.gmane.org/gmane.comp.version-control.git/297002
[2]
https://github.com/gitster/git/commit/efe472813d60befd72d6e2797934c90b22a26c93
[3] https://github.com/mhagger/git
---
refs/files-backend.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1230dfb..bbf96ad 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3393,7 +3393,8 @@ static const char *original_update_refname(struct
ref_update *update)
* - Lock the reference referred to by update.
* - Read the reference under lock.
* - Check that its old SHA-1 value (if specified) is correct, and in
- * any case record it for later use in the reflog.
+ * any case record it in update->lock->old_oid for later use when
+ * writing the reflog.
* - If it is a symref update without REF_NODEREF, split it up into a
* REF_LOG_ONLY update of the symref and add a separate update for
* the referent to transaction.
@@ -3556,7 +3557,6 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, i;
- struct ref_update **updates = transaction->updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
struct string_list_item *ref_to_delete;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
@@ -3582,7 +3582,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
* same refname as any existing ones.)
*/
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
struct string_list_item *item =
string_list_append(&affected_refnames, update->refname);
@@ -3632,7 +3632,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
* open at a time to avoid running out of file descriptors.
*/
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
ret = lock_ref_for_update(update, transaction, head_ref,
&affected_refnames, err);
@@ -3642,7 +3642,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
/* Perform updates first so live commits remain referenced */
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
struct ref_lock *lock = update->lock;
if (update->flags & REF_NEEDS_COMMIT ||
@@ -3674,7 +3674,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
}
/* Perform deletes now that updates are safely completed */
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
if (update->flags & REF_DELETING &&
!(update->flags & REF_LOG_ONLY)) {
@@ -3701,8 +3701,8 @@ cleanup:
transaction->state = REF_TRANSACTION_CLOSED;
for (i = 0; i < transaction->nr; i++)
- if (updates[i]->lock)
- unlock_ref(updates[i]->lock);
+ if (transaction->updates[i]->lock)
+ unlock_ref(transaction->updates[i]->lock);
string_list_clear(&refs_to_delete, 0);
free(head_ref);
string_list_clear(&affected_refnames, 0);
@@ -3722,7 +3722,6 @@ int initial_ref_transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, i;
- struct ref_update **updates = transaction->updates;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
assert(err);
@@ -3732,7 +3731,8 @@ int initial_ref_transaction_commit(struct ref_transaction
*transaction,
/* Fail if a refname appears more than once in the transaction: */
for (i = 0; i < transaction->nr; i++)
- string_list_append(&affected_refnames, updates[i]->refname);
+ string_list_append(&affected_refnames,
+ transaction->updates[i]->refname);
string_list_sort(&affected_refnames);
if (ref_update_reject_duplicates(&affected_refnames, err)) {
ret = TRANSACTION_GENERIC_ERROR;
@@ -3755,7 +3755,7 @@ int initial_ref_transaction_commit(struct ref_transaction
*transaction,
die("BUG: initial ref transaction called with existing refs");
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
if ((update->flags & REF_HAVE_OLD) &&
!is_null_sha1(update->old_sha1))
@@ -3776,7 +3776,7 @@ int initial_ref_transaction_commit(struct ref_transaction
*transaction,
}
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
if ((update->flags & REF_HAVE_NEW) &&
!is_null_sha1(update->new_sha1))
--
2.8.1
--
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
