This makes write_ref_sha1 only write the the lock file, committing
needs to be done outside of that function. This will help us change
the ref_transaction_commit in a later patch.

Also instead of calling unlock_ref before each return in write_ref_sha1
we can call this after the call. This is a first step to split up
write_ref_sha1 into the write and commit phase which is done in a
later patch.

There is a call in each code path after write_ref_sha1 now. Even in
the last hunk in the error case, the 'goto cleanup;' will make sure
there is a call to unlock_ref.

Signed-off-by: Stefan Beller <sbel...@google.com>
---

Notes:
    new in v3

 refs.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/refs.c b/refs.c
index f1eefc7..1bfc84b 100644
--- a/refs.c
+++ b/refs.c
@@ -2815,8 +2815,11 @@ static int close_ref(struct ref_lock *lock)
        return 0;
 }
 
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
 {
+       if (!lock->force_write && !hashcmp(lock->old_sha1, sha1))
+               return 0;
+
        if (commit_lock_file(lock->lk))
                return -1;
        return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
        }
        lock->force_write = 1;
        hashcpy(lock->old_sha1, orig_sha1);
-       if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+       if (write_ref_sha1(lock, orig_sha1, logmsg)
+           || commit_ref(lock, orig_sha1)) {
+               unlock_ref(lock);
                error("unable to write current sha1 into %s", newrefname);
                goto rollback;
        }
+       unlock_ref(lock);
 
        return 0;
 
@@ -2896,8 +2902,11 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
        lock->force_write = 1;
        flag = log_all_ref_updates;
        log_all_ref_updates = 0;
-       if (write_ref_sha1(lock, orig_sha1, NULL))
+       if (write_ref_sha1(lock, orig_sha1, NULL)
+           || commit_ref(lock, orig_sha1))
                error("unable to write current sha1 into %s", oldrefname);
+
+       unlock_ref(lock);
        log_all_ref_updates = flag;
 
  rollbacklog:
@@ -3067,22 +3076,19 @@ static int write_ref_sha1(struct ref_lock *lock,
                errno = EINVAL;
                return -1;
        }
-       if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
-               unlock_ref(lock);
+       if (!lock->force_write && !hashcmp(lock->old_sha1, sha1))
                return 0;
-       }
+
        o = parse_object(sha1);
        if (!o) {
                error("Trying to write ref %s with nonexistent object %s",
                        lock->ref_name, sha1_to_hex(sha1));
-               unlock_ref(lock);
                errno = EINVAL;
                return -1;
        }
        if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
                error("Trying to write non-commit object %s to branch %s",
                        sha1_to_hex(sha1), lock->ref_name);
-               unlock_ref(lock);
                errno = EINVAL;
                return -1;
        }
@@ -3091,7 +3097,6 @@ static int write_ref_sha1(struct ref_lock *lock,
            close_ref(lock) < 0) {
                int save_errno = errno;
                error("Couldn't write %s", lock->lk->filename.buf);
-               unlock_ref(lock);
                errno = save_errno;
                return -1;
        }
@@ -3099,7 +3104,6 @@ static int write_ref_sha1(struct ref_lock *lock,
        if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
            (strcmp(lock->ref_name, lock->orig_ref_name) &&
             log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 
0)) {
-               unlock_ref(lock);
                return -1;
        }
        if (strcmp(lock->orig_ref_name, "HEAD") != 0) {
@@ -3124,12 +3128,6 @@ static int write_ref_sha1(struct ref_lock *lock,
                    !strcmp(head_ref, lock->ref_name))
                        log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
        }
-       if (commit_ref(lock)) {
-               error("Couldn't set %s", lock->ref_name);
-               unlock_ref(lock);
-               return -1;
-       }
-       unlock_ref(lock);
        return 0;
 }
 
@@ -3762,14 +3760,15 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
                if (!is_null_sha1(update->new_sha1)) {
                        if (write_ref_sha1(update->lock, update->new_sha1,
-                                          update->msg)) {
-                               update->lock = NULL; /* freed by write_ref_sha1 
*/
+                                          update->msg)
+                           || commit_ref(update->lock, update->new_sha1)) {
                                strbuf_addf(err, "Cannot update the ref '%s'.",
                                            update->refname);
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
                        }
-                       update->lock = NULL; /* freed by write_ref_sha1 */
+                       unlock_ref(update->lock);
+                       update->lock = NULL;
                }
        }
 
@@ -4053,7 +4052,8 @@ int reflog_expire(const char *refname, const unsigned 
char *sha1,
                } else if (commit_lock_file(&reflog_lock)) {
                        status |= error("unable to commit reflog '%s' (%s)",
                                        log_file, strerror(errno));
-               } else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && 
commit_ref(lock)) {
+               } else if ((flags & EXPIRE_REFLOGS_UPDATE_REF)
+                           && commit_ref(lock, cb.last_kept_sha1)) {
                        status |= error("couldn't set %s", lock->ref_name);
                }
        }
-- 
2.2.1.62.g3f15098

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to