Merge branch 'jk/ident-ai-canonname-could-be-null'

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 47710fcf286283871c0949e888e4b319a7e83616..0709f60b8e8c43fa830c0e678fdf7b83435535bb 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1201,13 +1201,19 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
        struct strbuf refname;
        struct strbuf path = STRBUF_INIT;
        size_t path_baselen;
+       int err = 0;
 
        if (*refs->base.submodule)
-               strbuf_git_path_submodule(&path, refs->base.submodule, "%s", dirname);
+               err = strbuf_git_path_submodule(&path, refs->base.submodule, "%s", dirname);
        else
                strbuf_git_path(&path, "%s", dirname);
        path_baselen = path.len;
 
+       if (err) {
+               strbuf_release(&path);
+               return;
+       }
+
        d = opendir(path.buf);
        if (!d) {
                strbuf_release(&path);
@@ -1942,14 +1948,14 @@ static int verify_lock(struct ref_lock *lock,
                        errno = save_errno;
                        return -1;
                } else {
-                       hashclr(lock->old_oid.hash);
+                       oidclr(&lock->old_oid);
                        return 0;
                }
        }
        if (old_sha1 && 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),
+                           oid_to_hex(&lock->old_oid),
                            sha1_to_hex(old_sha1));
                errno = EBUSY;
                return -1;
@@ -3484,6 +3490,38 @@ static const char *original_update_refname(struct ref_update *update)
        return update->refname;
 }
 
+/*
+ * 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.
@@ -3531,7 +3569,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 
                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;
        }
@@ -3545,28 +3583,17 @@ static int lock_ref_for_update(struct files_ref_store *refs,
                         * 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(referent.buf, 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
@@ -3584,6 +3611,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,
        } 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
@@ -3595,20 +3625,6 @@ static int lock_ref_for_update(struct files_ref_store *refs,
                        struct ref_lock *parent_lock = parent_update->backend_data;
                        oidcpy(&parent_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) &&
@@ -3630,7 +3646,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
                         */
                        update->backend_data = 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;