To aid the effort, extract a new function, check_old_oid(), and use it
in the two places where the read value of the reference has to be
checked against update->old_sha1.
Update tests to reflect the improvements.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 77 ++--
t/t1404-update-ref-errors.sh | 14
2 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1230dfb..98c8b95 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3389,6 +3389,41 @@ 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 the result read for the reference. error is
+ * true iff there was an error reading the reference; otherwise, oid
+ * is the value read for the reference.
+ *
+ * If there was a problem, 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.
@@ -3432,7 +3467,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;
}
@@ -3446,28 +3481,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(update->refname, 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
@@ -3484,6 +3508,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 updat
