delete_ref(): handle special case more explicitly
[gitweb.git] / refs.c
diff --git a/refs.c b/refs.c
index 162760c0875d3c07887b2e0ce756b2dd0eb6c160..9b7bdd4198efe43d4eec0c149e7d3127ffd354cc 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -10,7 +10,7 @@ struct ref_lock {
        char *ref_name;
        char *orig_ref_name;
        struct lock_file *lk;
-       unsigned char old_sha1[20];
+       struct object_id old_oid;
 };
 
 /*
@@ -1757,7 +1757,7 @@ int ref_exists(const char *refname)
 }
 
 static int filter_refs(const char *refname, const struct object_id *oid,
-                      int flags, void *data)
+                          int flags, void *data)
 {
        struct ref_filter *filter = (struct ref_filter *)data;
 
@@ -1903,12 +1903,12 @@ static int warn_if_dangling_symref(const char *refname, const struct object_id *
 {
        struct warn_if_dangling_data *d = cb_data;
        const char *resolves_to;
-       unsigned char junk[20];
+       struct object_id junk;
 
        if (!(flags & REF_ISSYMREF))
                return 0;
 
-       resolves_to = resolve_ref_unsafe(refname, 0, junk, NULL);
+       resolves_to = resolve_ref_unsafe(refname, 0, junk.hash, NULL);
        if (!resolves_to
            || (d->refname
                ? strcmp(resolves_to, d->refname)
@@ -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_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)) {
-               error("Ref %s is at %s but expected %s", lock->ref_name,
-                       sha1_to_hex(lock->old_sha1), sha1_to_hex(old_sha1));
-               unlock_ref(lock);
+       if (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),
+                           sha1_to_hex(old_sha1));
                errno = EBUSY;
-               return NULL;
+               return -1;
        }
-       return lock;
+       return 0;
 }
 
 static int remove_empty_directories(const char *file)
@@ -2382,7 +2390,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        }
 
        refname = resolve_ref_unsafe(refname, resolve_flags,
-                                    lock->old_sha1, &type);
+                                    lock->old_oid.hash, &type);
        if (!refname && errno == EISDIR) {
                /* we are trying to lock foo but we used to
                 * have foo/bar which now does not exist;
@@ -2401,7 +2409,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                        goto error_return;
                }
                refname = resolve_ref_unsafe(orig_refname, resolve_flags,
-                                            lock->old_sha1, &type);
+                                            lock->old_oid.hash, &type);
        }
        if (type_p)
            *type_p = type;
@@ -2421,7 +2429,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
         * refname, nor a packed ref whose name is a proper prefix of
         * our refname.
         */
-       if (is_null_sha1(lock->old_sha1) &&
+       if (is_null_oid(&lock->old_oid) &&
            verify_refname_available(refname, extras, skip,
                                     get_packed_refs(&ref_cache), err)) {
                last_errno = ENOTDIR;
@@ -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);
@@ -2789,15 +2801,23 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
        return 0;
 }
 
-int delete_ref(const char *refname, const unsigned char *sha1, unsigned int flags)
+int delete_ref(const char *refname, const unsigned char *old_sha1,
+              unsigned int flags)
 {
        struct ref_transaction *transaction;
        struct strbuf err = STRBUF_INIT;
 
+       /*
+        * Treat NULL_SHA1 and NULL alike, to mean "we don't care what
+        * the old value of the reference was (or even if it didn't
+        * exist)":
+        */
+       if (old_sha1 && is_null_sha1(old_sha1))
+               old_sha1 = NULL;
+
        transaction = ref_transaction_begin(&err);
        if (!transaction ||
-           ref_transaction_delete(transaction, refname,
-                                  (sha1 && !is_null_sha1(sha1)) ? sha1 : NULL,
+           ref_transaction_delete(transaction, refname, old_sha1,
                                   flags, NULL, &err) ||
            ref_transaction_commit(transaction, &err)) {
                error("%s", err.buf);
@@ -2944,7 +2964,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
                strbuf_release(&err);
                goto rollback;
        }
-       hashcpy(lock->old_sha1, orig_sha1);
+       hashcpy(lock->old_oid.hash, orig_sha1);
 
        if (write_ref_to_lockfile(lock, orig_sha1) ||
            commit_ref_update(lock, orig_sha1, logmsg)) {
@@ -3199,9 +3219,9 @@ static int commit_ref_update(struct ref_lock *lock,
                             const unsigned char *sha1, const char *logmsg)
 {
        clear_loose_ref_cache(&ref_cache);
-       if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
+       if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg) < 0 ||
            (strcmp(lock->ref_name, lock->orig_ref_name) &&
-            log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
+            log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg) < 0)) {
                unlock_ref(lock);
                return -1;
        }
@@ -3225,7 +3245,7 @@ static int commit_ref_update(struct ref_lock *lock,
                                              head_sha1, &head_flag);
                if (head_ref && (head_flag & REF_ISSYMREF) &&
                    !strcmp(head_ref, lock->ref_name))
-                       log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
+                       log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg);
        }
        if (commit_ref(lock)) {
                error("Couldn't set %s", lock->ref_name);
@@ -3912,7 +3932,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;
@@ -3923,7 +3943,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                                                  (update->flags & REF_NODEREF));
 
                        if (!overwriting_symref &&
-                           !hashcmp(update->lock->old_sha1, update->new_sha1)) {
+                           !hashcmp(update->lock->old_oid.hash, update->new_sha1)) {
                                /*
                                 * The reference already has the desired
                                 * value, so we don't need to write it.
@@ -3935,7 +3955,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;