[PATCH 2/5] refs.c: remove unlock_ref from write_ref_sha1

[Stefan Beller <sbeller@xxxxxxxxxx>]

Instead of calling unlock_ref before each return in write_ref_sha1
we can call this after the call. This is a first step to split up
write_ref_sha1 into the write and commit phase which is done in a
later patch.

There is a call in each code path after write_ref_sha1 now. Even in
the last hunk in the error case, the 'goto cleanup;' will make sure
there is a call to unlock_ref.

Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
---
 refs.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index aae3b66..4580919 100644
--- a/refs.c
+++ b/refs.c
@@ -2866,9 +2866,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
 	lock->force_write = 1;
 	hashcpy(lock->old_sha1, orig_sha1);
 	if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+		unlock_ref(lock);
 		error("unable to write current sha1 into %s", newrefname);
 		goto rollback;
 	}
+	unlock_ref(lock);
 
 	return 0;
 
@@ -2884,6 +2886,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
 	log_all_ref_updates = 0;
 	if (write_ref_sha1(lock, orig_sha1, NULL))
 		error("unable to write current sha1 into %s", oldrefname);
+	unlock_ref(lock);
 	log_all_ref_updates = flag;
 
  rollbacklog:
@@ -3083,22 +3086,19 @@ static int write_ref_sha1(struct ref_lock *lock,
 		errno = EINVAL;
 		return -1;
 	}
-	if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
-		unlock_ref(lock);
+	if (!lock->force_write && !hashcmp(lock->old_sha1, sha1))
 		return 0;
-	}
+
 	o = parse_object(sha1);
 	if (!o) {
 		error("Trying to write ref %s with nonexistent object %s",
 			lock->ref_name, sha1_to_hex(sha1));
-		unlock_ref(lock);
 		errno = EINVAL;
 		return -1;
 	}
 	if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
 		error("Trying to write non-commit object %s to branch %s",
 			sha1_to_hex(sha1), lock->ref_name);
-		unlock_ref(lock);
 		errno = EINVAL;
 		return -1;
 	}
@@ -3106,7 +3106,6 @@ static int write_ref_sha1(struct ref_lock *lock,
 	    close_ref(lock) < 0) {
 		int save_errno = errno;
 		error("Couldn't write %s", lock->lk->filename.buf);
-		unlock_ref(lock);
 		errno = save_errno;
 		return -1;
 	}
@@ -3114,7 +3113,6 @@ static int write_ref_sha1(struct ref_lock *lock,
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
 	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
-		unlock_ref(lock);
 		return -1;
 	}
 	if (strcmp(lock->orig_ref_name, "HEAD") != 0) {
@@ -3141,10 +3139,8 @@ static int write_ref_sha1(struct ref_lock *lock,
 	}
 	if (commit_ref(lock)) {
 		error("Couldn't set %s", lock->ref_name);
-		unlock_ref(lock);
 		return -1;
 	}
-	unlock_ref(lock);
 	return 0;
 }
 
@@ -3770,7 +3766,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 			goto cleanup;
 		}
 		/* Do not keep all lock files open at the same time. */
-		close_lock_file(update->lock->lk);
+		close_ref(update->lock);
 	}
 
 	/* Perform updates first so live commits remain referenced */
@@ -3780,13 +3776,13 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 		if (!is_null_sha1(update->new_sha1)) {
 			if (write_ref_sha1(update->lock, update->new_sha1,
 					   update->msg)) {
-				update->lock = NULL; /* freed by write_ref_sha1 */
 				strbuf_addf(err, "Cannot update the ref '%s'.",
 					    update->refname);
 				ret = TRANSACTION_GENERIC_ERROR;
 				goto cleanup;
 			}
-			update->lock = NULL; /* freed by write_ref_sha1 */
+			unlock_ref(update->lock);
+			update->lock = NULL;
 		}
 	}
 
-- 
2.2.1.62.g3f15098

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