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 <sbeller@xxxxxxxxxx> --- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html