Merge branch 'mh/verify-lock-error-report'
authorJunio C Hamano <gitster@pobox.com>
Thu, 11 Jun 2015 16:29:54 +0000 (09:29 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 11 Jun 2015 16:29:54 +0000 (09:29 -0700)
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 *

1  2 
refs.c
diff --cc refs.c
index a742d7925dfbd08ff7623427b74183406fdb85ec,6ae81e97442568891c4faf29025b55c68029477d..26d1ac1e32eb4fc4c6729abcf0354dbad6177d1b
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -2219,27 -2218,35 +2219,35 @@@ static void unlock_ref(struct ref_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_sha1, NULL)) {
 +                        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_sha1, old_sha1)) {
 +      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_sha1),
++                          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)