diff options
author | Michael Haggerty <mhagger@alum.mit.edu> | 2016-06-07 09:29:23 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2016-06-20 11:49:00 -0700 |
commit | e3f510393c9d373f2969badc2b8afe179803a0fa (patch) | |
tree | 57c2b89dbd15141a4389ad0c463c28a9762c1d28 /refs | |
parent | t1404: add more tests of update-ref error handling (diff) | |
download | tgif-e3f510393c9d373f2969badc2b8afe179803a0fa.tar.xz |
lock_ref_for_update(): make error handling more uniform
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 <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'refs')
-rw-r--r-- | refs/files-backend.c | 74 |
1 files changed, 42 insertions, 32 deletions
diff --git a/refs/files-backend.c b/refs/files-backend.c index bbf96ad83a..6f8fecd024 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3389,6 +3389,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. @@ -3433,7 +3465,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; } @@ -3447,28 +3479,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 @@ -3485,6 +3506,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 @@ -3495,20 +3519,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) && @@ -3530,7 +3540,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; |