1 files changed, 44 insertions, 34 deletions

diff --git a/refs/files-backend.c b/refs/files-backend.c
index a9ca003bae..12290d2496 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2030,14 +2030,14 @@ static int verify_lock(struct ref_lock *lock,
 			errno = save_errno;
 			return -1;
 		} else {
-			hashclr(lock->old_oid.hash);
+			oidclr(&lock->old_oid);
 			return 0;
 		}
 	}
 	if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
 		strbuf_addf(err, "ref '%s' is at %s but expected %s",
 			    lock->ref_name,
-			    sha1_to_hex(lock->old_oid.hash),
+			    oid_to_hex(&lock->old_oid),
 			    sha1_to_hex(old_sha1));
 		errno = EBUSY;
 		return -1;
@@ -3518,6 +3518,38 @@ static const char *original_update_refname(struct ref_update *update)
 }
 
 /*
+ * Check whether the REF_HAVE_OLD and old_oid values stored in update
+ * are consistent with oid, which is the reference's current value. If
+ * everything is OK, return 0; otherwise, write an error message to
+ * err and return -1.
+ */
+static int check_old_oid(struct ref_update *update, struct object_id *oid,
+			 struct strbuf *err)
+{
+	if (!(update->flags & REF_HAVE_OLD) ||
+		   !hashcmp(oid->hash, update->old_sha1))
+		return 0;
+
+	if (is_null_sha1(update->old_sha1))
+		strbuf_addf(err, "cannot lock ref '%s': "
+			    "reference already exists",
+			    original_update_refname(update));
+	else if (is_null_oid(oid))
+		strbuf_addf(err, "cannot lock ref '%s': "
+			    "reference is missing but expected %s",
+			    original_update_refname(update),
+			    sha1_to_hex(update->old_sha1));
+	else
+		strbuf_addf(err, "cannot lock ref '%s': "
+			    "is at %s but expected %s",
+			    original_update_refname(update),
+			    oid_to_hex(oid),
+			    sha1_to_hex(update->old_sha1));
+
+	return -1;
+}
+
+/*
  * Prepare for carrying out update:
  * - Lock the reference referred to by update.
  * - Read the reference under lock.
@@ -3562,7 +3594,7 @@ static int lock_ref_for_update(struct ref_update *update,
 
 		reason = strbuf_detach(err, NULL);
 		strbuf_addf(err, "cannot lock ref '%s': %s",
-			    update->refname, reason);
+			    original_update_refname(update), reason);
 		free(reason);
 		return ret;
 	}
@@ -3576,28 +3608,17 @@ static int lock_ref_for_update(struct ref_update *update,
 			 * the transaction, so we have to read it here
 			 * to record and possibly check old_sha1:
 			 */
-			if (read_ref_full(update->refname,
-					  mustexist ? RESOLVE_REF_READING : 0,
+			if (read_ref_full(referent.buf, 0,
 					  lock->old_oid.hash, NULL)) {
 				if (update->flags & REF_HAVE_OLD) {
 					strbuf_addf(err, "cannot lock ref '%s': "
-						    "can't resolve old value",
-						    update->refname);
-					return TRANSACTION_GENERIC_ERROR;
-				} else {
-					hashclr(lock->old_oid.hash);
+						    "error reading reference",
+						    original_update_refname(update));
+					return -1;
 				}
-			}
-			if ((update->flags & REF_HAVE_OLD) &&
-			    hashcmp(lock->old_oid.hash, update->old_sha1)) {
-				strbuf_addf(err, "cannot lock ref '%s': "
-					    "is at %s but expected %s",
-					    update->refname,
-					    sha1_to_hex(lock->old_oid.hash),
-					    sha1_to_hex(update->old_sha1));
+			} else if (check_old_oid(update, &lock->old_oid, err)) {
 				return TRANSACTION_GENERIC_ERROR;
 			}
-
 		} else {
 			/*
 			 * Create a new update for the reference this
@@ -3614,6 +3635,9 @@ static int lock_ref_for_update(struct ref_update *update,
 	} else {
 		struct ref_update *parent_update;
 
+		if (check_old_oid(update, &lock->old_oid, err))
+			return TRANSACTION_GENERIC_ERROR;
+
 		/*
 		 * If this update is happening indirectly because of a
 		 * symref update, record the old SHA-1 in the parent
@@ -3624,20 +3648,6 @@ static int lock_ref_for_update(struct ref_update *update,
 		     parent_update = parent_update->parent_update) {
 			oidcpy(&parent_update->lock->old_oid, &lock->old_oid);
 		}
-
-		if ((update->flags & REF_HAVE_OLD) &&
-		    hashcmp(lock->old_oid.hash, update->old_sha1)) {
-			if (is_null_sha1(update->old_sha1))
-				strbuf_addf(err, "cannot lock ref '%s': reference already exists",
-					    original_update_refname(update));
-			else
-				strbuf_addf(err, "cannot lock ref '%s': is at %s but expected %s",
-					    original_update_refname(update),
-					    sha1_to_hex(lock->old_oid.hash),
-					    sha1_to_hex(update->old_sha1));
-
-			return TRANSACTION_GENERIC_ERROR;
-		}
 	}
 
 	if ((update->flags & REF_HAVE_NEW) &&
@@ -3659,7 +3669,7 @@ static int lock_ref_for_update(struct ref_update *update,
 			 */
 			update->lock = NULL;
 			strbuf_addf(err,
-				    "cannot update the ref '%s': %s",
+				    "cannot update ref '%s': %s",
 				    update->refname, write_err);
 			free(write_err);
 			return TRANSACTION_GENERIC_ERROR;