verify_lock(): return 0/-1 rather than struct ref_lock *
[gitweb.git] / refs.c
diff --git a/refs.c b/refs.c
index 43e7bdd896469b846b3c53c873f5ae3ebc2a1590..27b769afddb97bd9ab6611cf16761666093ae23c 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -11,7 +11,6 @@ struct ref_lock {
        char *orig_ref_name;
        struct lock_file *lk;
        unsigned char old_sha1[20];
-       int lock_fd;
 };
 
 /*
@@ -57,6 +56,12 @@ static unsigned char refname_disposition[256] = {
  */
 #define REF_HAVE_OLD   0x10
 
+/*
+ * Used as a flag in ref_update::flags when the lockfile needs to be
+ * committed.
+ */
+#define REF_NEEDS_COMMIT 0x20
+
 /*
  * Try to read one refname component from the front of refname.
  * Return the length of the component found, or -1 if the component is
@@ -344,8 +349,6 @@ static struct ref_entry *create_ref_entry(const char *refname,
        if (check_name &&
            check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
                die("Reference has invalid format: '%s'", refname);
-       if (!check_name && !refname_is_safe(refname))
-               die("Reference has invalid name: '%s'", refname);
        len = strlen(refname) + 1;
        ref = xmalloc(sizeof(struct ref_entry) + len);
        hashcpy(ref->u.value.sha1, sha1);
@@ -1238,6 +1241,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
                        int flag = REF_ISPACKED;
 
                        if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+                               if (!refname_is_safe(refname))
+                                       die("packed refname is dangerous: %s", refname);
                                hashclr(sha1);
                                flag |= REF_BAD_NAME | REF_ISBROKEN;
                        }
@@ -1383,6 +1388,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
                        }
                        if (check_refname_format(refname.buf,
                                                 REFNAME_ALLOW_ONELEVEL)) {
+                               if (!refname_is_safe(refname.buf))
+                                       die("loose refname is dangerous: %s", refname.buf);
                                hashclr(sha1);
                                flag |= REF_BAD_NAME | REF_ISBROKEN;
                        }
@@ -1442,7 +1449,7 @@ static int resolve_gitlink_ref_recursive(struct ref_cache *refs,
 {
        int fd, len;
        char buffer[128], *p;
-       char *path;
+       const char *path;
 
        if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)
                return -1;
@@ -1535,7 +1542,11 @@ static int resolve_missing_loose_ref(const char *refname,
 }
 
 /* This function needs to return a meaningful errno on failure */
-const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)
+static const char *resolve_ref_unsafe_1(const char *refname,
+                                       int resolve_flags,
+                                       unsigned char *sha1,
+                                       int *flags,
+                                       struct strbuf *sb_path)
 {
        int depth = MAXDEPTH;
        ssize_t len;
@@ -1566,7 +1577,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
                bad_name = 1;
        }
        for (;;) {
-               char path[PATH_MAX];
+               const char *path;
                struct stat st;
                char *buf;
                int fd;
@@ -1576,7 +1587,9 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
                        return NULL;
                }
 
-               git_snpath(path, sizeof(path), "%s", refname);
+               strbuf_reset(sb_path);
+               strbuf_git_path(sb_path, "%s", refname);
+               path = sb_path->buf;
 
                /*
                 * We might have to loop back here to avoid a race
@@ -1703,6 +1716,16 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
        }
 }
 
+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
+                              unsigned char *sha1, int *flags)
+{
+       struct strbuf sb_path = STRBUF_INIT;
+       const char *ret = resolve_ref_unsafe_1(refname, resolve_flags,
+                                              sha1, flags, &sb_path);
+       strbuf_release(&sb_path);
+       return ret;
+}
+
 char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags)
 {
        return xstrdup_or_null(resolve_ref_unsafe(ref, resolve_flags, sha1, flags));
@@ -2195,9 +2218,14 @@ 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 or a negative value on error. This function should make
+ * sure errno is meaningful on error.
+ */
+static int verify_lock(struct ref_lock *lock,
+                      const unsigned char *old_sha1, int mustexist)
 {
        if (read_ref_full(lock->ref_name,
                          mustexist ? RESOLVE_REF_READING : 0,
@@ -2206,16 +2234,16 @@ static struct ref_lock *verify_lock(struct ref_lock *lock,
                error("Can't verify ref %s", lock->ref_name);
                unlock_ref(lock);
                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);
                errno = EBUSY;
-               return NULL;
+               return -1;
        }
-       return lock;
+       return 0;
 }
 
 static int remove_empty_directories(const char *file)
@@ -2333,9 +2361,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                                            const unsigned char *old_sha1,
                                            const struct string_list *extras,
                                            const struct string_list *skip,
-                                           unsigned int flags, int *type_p)
+                                           unsigned int flags, int *type_p,
+                                           struct strbuf *err)
 {
-       char *ref_file;
+       const char *ref_file;
        const char *orig_refname = refname;
        struct ref_lock *lock;
        int last_errno = 0;
@@ -2343,10 +2372,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
        int resolve_flags = 0;
        int attempts_remaining = 3;
-       struct strbuf err = STRBUF_INIT;
+
+       assert(err);
 
        lock = xcalloc(1, sizeof(struct ref_lock));
-       lock->lock_fd = -1;
 
        if (mustexist)
                resolve_flags |= RESOLVE_REF_READING;
@@ -2367,7 +2396,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                ref_file = git_path("%s", orig_refname);
                if (remove_empty_directories(ref_file)) {
                        last_errno = errno;
-                       error("there are still refs under '%s'", orig_refname);
+
+                       if (!verify_refname_available(orig_refname, extras, skip,
+                                                     get_loose_refs(&ref_cache), err))
+                               strbuf_addf(err, "there are still refs under '%s'",
+                                           orig_refname);
+
                        goto error_return;
                }
                refname = resolve_ref_unsafe(orig_refname, resolve_flags,
@@ -2377,8 +2411,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
            *type_p = type;
        if (!refname) {
                last_errno = errno;
-               error("unable to resolve reference %s: %s",
-                       orig_refname, strerror(errno));
+               if (last_errno != ENOTDIR ||
+                   !verify_refname_available(orig_refname, extras, skip,
+                                             get_loose_refs(&ref_cache), err))
+                       strbuf_addf(err, "unable to resolve reference %s: %s",
+                                   orig_refname, strerror(last_errno));
+
                goto error_return;
        }
        /*
@@ -2389,8 +2427,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
         */
        if (is_null_sha1(lock->old_sha1) &&
            verify_refname_available(refname, extras, skip,
-                                    get_packed_refs(&ref_cache), &err)) {
-               error("%s", err.buf);
+                                    get_packed_refs(&ref_cache), err)) {
                last_errno = ENOTDIR;
                goto error_return;
        }
@@ -2407,7 +2444,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        ref_file = git_path("%s", refname);
 
  retry:
-       switch (safe_create_leading_directories(ref_file)) {
+       switch (safe_create_leading_directories_const(ref_file)) {
        case SCLD_OK:
                break; /* success */
        case SCLD_VANISHED:
@@ -2416,12 +2453,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                /* fall through */
        default:
                last_errno = errno;
-               error("unable to create directory for %s", ref_file);
+               strbuf_addf(err, "unable to create directory for %s", ref_file);
                goto error_return;
        }
 
-       lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
-       if (lock->lock_fd < 0) {
+       if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
                last_errno = errno;
                if (errno == ENOENT && --attempts_remaining > 0)
                        /*
@@ -2431,16 +2467,16 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                         */
                        goto retry;
                else {
-                       unable_to_lock_message(ref_file, errno, &err);
-                       error("%s", err.buf);
+                       unable_to_lock_message(ref_file, errno, err);
                        goto error_return;
                }
        }
-       return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
+       if (old_sha1 && verify_lock(lock, old_sha1, mustexist))
+               return NULL;
+       return lock;
 
  error_return:
        unlock_ref(lock);
-       strbuf_release(&err);
        errno = last_errno;
        return NULL;
 }
@@ -2476,9 +2512,19 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
 /* This should return a meaningful errno on failure */
 int lock_packed_refs(int flags)
 {
+       static int timeout_configured = 0;
+       static int timeout_value = 1000;
+
        struct packed_ref_cache *packed_ref_cache;
 
-       if (hold_lock_file_for_update(&packlock, git_path("packed-refs"), flags) < 0)
+       if (!timeout_configured) {
+               git_config_get_int("core.packedrefstimeout", &timeout_value);
+               timeout_configured = 1;
+       }
+
+       if (hold_lock_file_for_update_timeout(
+                           &packlock, git_path("packed-refs"),
+                           flags, timeout_value) < 0)
                return -1;
        /*
         * Get the current packed-refs while holding the lock.  If the
@@ -2784,7 +2830,7 @@ static int rename_tmp_log(const char *newrefname)
        int attempts_remaining = 4;
 
  retry:
-       switch (safe_create_leading_directories(git_path("logs/%s", newrefname))) {
+       switch (safe_create_leading_directories_const(git_path("logs/%s", newrefname))) {
        case SCLD_OK:
                break; /* success */
        case SCLD_VANISHED:
@@ -2843,8 +2889,9 @@ static int rename_ref_available(const char *oldname, const char *newname)
        return ret;
 }
 
-static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
-                         const char *logmsg);
+static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1);
+static int commit_ref_update(struct ref_lock *lock,
+                            const unsigned char *sha1, const char *logmsg);
 
 int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
 {
@@ -2854,6 +2901,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
        struct stat loginfo;
        int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
        const char *symref = NULL;
+       struct strbuf err = STRBUF_INIT;
 
        if (log && S_ISLNK(loginfo.st_mode))
                return error("reflog for %s is a symlink", oldrefname);
@@ -2896,13 +2944,16 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
 
        logmoved = log;
 
-       lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, 0, NULL);
+       lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, 0, NULL, &err);
        if (!lock) {
-               error("unable to lock %s for update", newrefname);
+               error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+               strbuf_release(&err);
                goto rollback;
        }
        hashcpy(lock->old_sha1, orig_sha1);
-       if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+
+       if (write_ref_to_lockfile(lock, orig_sha1) ||
+           commit_ref_update(lock, orig_sha1, logmsg)) {
                error("unable to write current sha1 into %s", newrefname);
                goto rollback;
        }
@@ -2910,15 +2961,17 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
        return 0;
 
  rollback:
-       lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, 0, NULL);
+       lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, 0, NULL, &err);
        if (!lock) {
-               error("unable to lock %s for rollback", oldrefname);
+               error("unable to lock %s for rollback: %s", oldrefname, err.buf);
+               strbuf_release(&err);
                goto rollbacklog;
        }
 
        flag = log_all_ref_updates;
        log_all_ref_updates = 0;
-       if (write_ref_sha1(lock, orig_sha1, NULL))
+       if (write_ref_to_lockfile(lock, orig_sha1) ||
+           commit_ref_update(lock, orig_sha1, NULL))
                error("unable to write current sha1 into %s", oldrefname);
        log_all_ref_updates = flag;
 
@@ -2938,7 +2991,6 @@ static int close_ref(struct ref_lock *lock)
 {
        if (close_lock_file(lock->lk))
                return -1;
-       lock->lock_fd = -1;
        return 0;
 }
 
@@ -2946,7 +2998,6 @@ static int commit_ref(struct ref_lock *lock)
 {
        if (commit_lock_file(lock->lk))
                return -1;
-       lock->lock_fd = -1;
        return 0;
 }
 
@@ -2977,11 +3028,15 @@ static int copy_msg(char *buf, const char *msg)
 }
 
 /* This function must set a meaningful errno on failure */
-int log_ref_setup(const char *refname, char *logfile, int bufsize)
+int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
 {
        int logfd, oflags = O_APPEND | O_WRONLY;
+       char *logfile;
 
-       git_snpath(logfile, bufsize, "logs/%s", refname);
+       strbuf_git_path(sb_logfile, "logs/%s", refname);
+       logfile = sb_logfile->buf;
+       /* make sure the rest of the function can't change "logfile" */
+       sb_logfile = NULL;
        if (log_all_ref_updates &&
            (starts_with(refname, "refs/heads/") ||
             starts_with(refname, "refs/remotes/") ||
@@ -3052,18 +3107,22 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
        return 0;
 }
 
-static int log_ref_write(const char *refname, const unsigned char *old_sha1,
-                        const unsigned char *new_sha1, const char *msg)
+static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
+                          const unsigned char *new_sha1, const char *msg,
+                          struct strbuf *sb_log_file)
 {
        int logfd, result, oflags = O_APPEND | O_WRONLY;
-       char log_file[PATH_MAX];
+       char *log_file;
 
        if (log_all_ref_updates < 0)
                log_all_ref_updates = !is_bare_repository();
 
-       result = log_ref_setup(refname, log_file, sizeof(log_file));
+       result = log_ref_setup(refname, sb_log_file);
        if (result)
                return result;
+       log_file = sb_log_file->buf;
+       /* make sure the rest of the function can't change "log_file" */
+       sb_log_file = NULL;
 
        logfd = open(log_file, oflags);
        if (logfd < 0)
@@ -3086,17 +3145,26 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
        return 0;
 }
 
+static int log_ref_write(const char *refname, const unsigned char *old_sha1,
+                        const unsigned char *new_sha1, const char *msg)
+{
+       struct strbuf sb = STRBUF_INIT;
+       int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb);
+       strbuf_release(&sb);
+       return ret;
+}
+
 int is_branch(const char *refname)
 {
        return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
 }
 
 /*
- * Write sha1 into the ref specified by the lock. Make sure that errno
- * is sane on error.
+ * Write sha1 into the open lockfile, then close the lockfile. On
+ * errors, rollback the lockfile and set errno to reflect the problem.
  */
-static int write_ref_sha1(struct ref_lock *lock,
-       const unsigned char *sha1, const char *logmsg)
+static int write_ref_to_lockfile(struct ref_lock *lock,
+                                const unsigned char *sha1)
 {
        static char term = '\n';
        struct object *o;
@@ -3116,8 +3184,8 @@ static int write_ref_sha1(struct ref_lock *lock,
                errno = EINVAL;
                return -1;
        }
-       if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
-           write_in_full(lock->lock_fd, &term, 1) != 1 ||
+       if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
+           write_in_full(lock->lk->fd, &term, 1) != 1 ||
            close_ref(lock) < 0) {
                int save_errno = errno;
                error("Couldn't write %s", lock->lk->filename.buf);
@@ -3125,6 +3193,17 @@ static int write_ref_sha1(struct ref_lock *lock,
                errno = save_errno;
                return -1;
        }
+       return 0;
+}
+
+/*
+ * Commit a change to a loose reference that has already been written
+ * to the loose reference lockfile. Also update the reflogs if
+ * necessary, using the specified lockmsg (which can be NULL).
+ */
+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 ||
            (strcmp(lock->ref_name, lock->orig_ref_name) &&
@@ -3811,56 +3890,92 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                goto cleanup;
        }
 
-       /* Acquire all locks while verifying old values */
+       /*
+        * Acquire all locks, verify old values if provided, check
+        * that new values are valid, and write new values to the
+        * lockfiles, ready to be activated. Only keep one lockfile
+        * open at a time to avoid running out of file descriptors.
+        */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
-               unsigned int flags = update->flags;
 
-               if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
-                       flags |= REF_DELETING;
+               if ((update->flags & REF_HAVE_NEW) &&
+                   is_null_sha1(update->new_sha1))
+                       update->flags |= REF_DELETING;
                update->lock = lock_ref_sha1_basic(
                                update->refname,
                                ((update->flags & REF_HAVE_OLD) ?
                                 update->old_sha1 : NULL),
                                &affected_refnames, NULL,
-                               flags,
-                               &update->type);
+                               update->flags,
+                               &update->type,
+                               err);
                if (!update->lock) {
+                       char *reason;
+
                        ret = (errno == ENOTDIR)
                                ? TRANSACTION_NAME_CONFLICT
                                : TRANSACTION_GENERIC_ERROR;
-                       strbuf_addf(err, "Cannot lock the ref '%s'.",
-                                   update->refname);
+                       reason = strbuf_detach(err, NULL);
+                       strbuf_addf(err, "Cannot lock ref '%s': %s",
+                                   update->refname, reason);
+                       free(reason);
                        goto cleanup;
                }
-       }
-
-       /* Perform updates first so live commits remain referenced */
-       for (i = 0; i < n; i++) {
-               struct ref_update *update = updates[i];
-               int flags = update->flags;
-
-               if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) {
+               if ((update->flags & REF_HAVE_NEW) &&
+                   !(update->flags & REF_DELETING)) {
                        int overwriting_symref = ((update->type & REF_ISSYMREF) &&
                                                  (update->flags & REF_NODEREF));
 
-                       if (!overwriting_symref
-                           && !hashcmp(update->lock->old_sha1, update->new_sha1)) {
+                       if (!overwriting_symref &&
+                           !hashcmp(update->lock->old_sha1, update->new_sha1)) {
                                /*
                                 * The reference already has the desired
                                 * value, so we don't need to write it.
                                 */
-                               unlock_ref(update->lock);
+                       } else if (write_ref_to_lockfile(update->lock,
+                                                        update->new_sha1)) {
+                               /*
+                                * The lock was freed upon failure of
+                                * write_ref_to_lockfile():
+                                */
+                               update->lock = NULL;
+                               strbuf_addf(err, "Cannot update the ref '%s'.",
+                                           update->refname);
+                               ret = TRANSACTION_GENERIC_ERROR;
+                               goto cleanup;
+                       } else {
+                               update->flags |= REF_NEEDS_COMMIT;
+                       }
+               }
+               if (!(update->flags & REF_NEEDS_COMMIT)) {
+                       /*
+                        * We didn't have to write anything to the lockfile.
+                        * Close it to free up the file descriptor:
+                        */
+                       if (close_ref(update->lock)) {
+                               strbuf_addf(err, "Couldn't close %s.lock",
+                                           update->refname);
+                               goto cleanup;
+                       }
+               }
+       }
+
+       /* Perform updates first so live commits remain referenced */
+       for (i = 0; i < n; i++) {
+               struct ref_update *update = updates[i];
+
+               if (update->flags & REF_NEEDS_COMMIT) {
+                       if (commit_ref_update(update->lock,
+                                             update->new_sha1, update->msg)) {
+                               /* freed by commit_ref_update(): */
                                update->lock = NULL;
-                       } else if (write_ref_sha1(update->lock, update->new_sha1,
-                                                 update->msg)) {
-                               update->lock = NULL; /* freed by write_ref_sha1 */
                                strbuf_addf(err, "Cannot update the ref '%s'.",
                                            update->refname);
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
                        } else {
-                               /* freed by write_ref_sha1(): */
+                               /* freed by commit_ref_update(): */
                                update->lock = NULL;
                        }
                }
@@ -3869,15 +3984,14 @@ int ref_transaction_commit(struct ref_transaction *transaction,
        /* Perform deletes now that updates are safely completed */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
-               int flags = update->flags;
 
-               if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) {
+               if (update->flags & REF_DELETING) {
                        if (delete_ref_loose(update->lock, update->type, err)) {
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
                        }
 
-                       if (!(flags & REF_ISPRUNING))
+                       if (!(update->flags & REF_ISPRUNING))
                                string_list_append(&refs_to_delete,
                                                   update->lock->ref_name);
                }
@@ -4088,6 +4202,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
        char *log_file;
        int status = 0;
        int type;
+       struct strbuf err = STRBUF_INIT;
 
        memset(&cb, 0, sizeof(cb));
        cb.flags = flags;
@@ -4099,9 +4214,12 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
         * reference itself, plus we might need to update the
         * reference if --updateref was specified:
         */
-       lock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, 0, &type);
-       if (!lock)
-               return error("cannot lock ref '%s'", refname);
+       lock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, 0, &type, &err);
+       if (!lock) {
+               error("cannot lock ref '%s': %s", refname, err.buf);
+               strbuf_release(&err);
+               return -1;
+       }
        if (!reflog_exists(refname)) {
                unlock_ref(lock);
                return 0;
@@ -4151,9 +4269,9 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
                        status |= error("couldn't write %s: %s", log_file,
                                        strerror(errno));
                } else if (update &&
-                       (write_in_full(lock->lock_fd,
+                          (write_in_full(lock->lk->fd,
                                sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-                        write_str_in_full(lock->lock_fd, "\n") != 1 ||
+                        write_str_in_full(lock->lk->fd, "\n") != 1 ||
                         close_ref(lock) < 0)) {
                        status |= error("couldn't write %s",
                                        lock->lk->filename.buf);