Merge branch 'mh/verify-lock-error-report'

Bring consistency to error reporting mechanism used in "refs" API.

* mh/verify-lock-error-report:
  ref_transaction_commit(): do not capitalize error messages
  verify_lock(): do not capitalize error messages
  verify_lock(): report errors via a strbuf
  verify_lock(): on errors, let the caller unlock the lock
  verify_lock(): return 0/-1 rather than struct ref_lock *
This commit is contained in:
Junio C Hamano
2015-06-11 09:29:54 -07:00
2 changed files with 33 additions and 21 deletions

40
refs.c
View File

@ -2219,27 +2219,35 @@ static void unlock_ref(struct ref_lock *lock)
free(lock);
}
/* This function should make sure errno is meaningful on error */
static struct ref_lock *verify_lock(struct ref_lock *lock,
const unsigned char *old_sha1, int mustexist)
/*
* Verify that the reference locked by lock has the value old_sha1.
* Fail if the reference doesn't exist and mustexist is set. Return 0
* on success. On error, write an error message to err, set errno, and
* return a negative value.
*/
static int verify_lock(struct ref_lock *lock,
const unsigned char *old_sha1, int mustexist,
struct strbuf *err)
{
assert(err);
if (read_ref_full(lock->ref_name,
mustexist ? RESOLVE_REF_READING : 0,
lock->old_oid.hash, NULL)) {
int save_errno = errno;
error("Can't verify ref %s", lock->ref_name);
unlock_ref(lock);
strbuf_addf(err, "can't verify ref %s", lock->ref_name);
errno = save_errno;
return NULL;
return -1;
}
if (hashcmp(lock->old_oid.hash, old_sha1)) {
error("Ref %s is at %s but expected %s", lock->ref_name,
oid_to_hex(&lock->old_oid), sha1_to_hex(old_sha1));
unlock_ref(lock);
strbuf_addf(err, "ref %s is at %s but expected %s",
lock->ref_name,
sha1_to_hex(lock->old_oid.hash),
sha1_to_hex(old_sha1));
errno = EBUSY;
return NULL;
return -1;
}
return lock;
return 0;
}
static int remove_empty_directories(const char *file)
@ -2467,7 +2475,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
goto error_return;
}
}
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
if (old_sha1 && verify_lock(lock, old_sha1, mustexist, err)) {
last_errno = errno;
goto error_return;
}
return lock;
error_return:
unlock_ref(lock);
@ -3912,7 +3924,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
? TRANSACTION_NAME_CONFLICT
: TRANSACTION_GENERIC_ERROR;
reason = strbuf_detach(err, NULL);
strbuf_addf(err, "Cannot lock ref '%s': %s",
strbuf_addf(err, "cannot lock ref '%s': %s",
update->refname, reason);
free(reason);
goto cleanup;
@ -3935,7 +3947,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
* write_ref_to_lockfile():
*/
update->lock = NULL;
strbuf_addf(err, "Cannot update the ref '%s'.",
strbuf_addf(err, "cannot update the ref '%s'.",
update->refname);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;