Merge branch 'sb/ref-lock-lose-lock-fd'
authorJunio C Hamano <gitster@pobox.com>
Tue, 19 May 2015 20:17:59 +0000 (13:17 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 19 May 2015 20:17:59 +0000 (13:17 -0700)
The refs API uses ref_lock struct which had its own "int fd", even
though the same file descriptor was in the lock struct it contains.
Clean-up the code to lose this redundant field.

* sb/ref-lock-lose-lock-fd:
refs.c: remove lock_fd from struct ref_lock

1  2 
refs.c
diff --combined refs.c
index 81a455b807e4ef7f79e3cdb252eedfccfa3c7d8a,4f495bd7625b613bf20d45d3ad574b9c423faaea..b5189f4a574c555ced4f24bc0e12b2702ae2e5b4
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -11,7 -11,6 +11,6 @@@ struct ref_lock 
        char *orig_ref_name;
        struct lock_file *lk;
        unsigned char old_sha1[20];
-       int lock_fd;
  };
  
  /*
@@@ -344,6 -343,8 +343,6 @@@ static struct ref_entry *create_ref_ent
        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);
@@@ -1176,8 -1177,6 +1175,8 @@@ static void read_packed_refs(FILE *f, s
                        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;
                        }
@@@ -1323,8 -1322,6 +1322,8 @@@ static void read_loose_refs(const char 
                        }
                        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;
                        }
@@@ -1384,7 -1381,7 +1383,7 @@@ static int resolve_gitlink_ref_recursiv
  {
        int fd, len;
        char buffer[128], *p;
 -      char *path;
 +      const char *path;
  
        if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)
                return -1;
@@@ -1477,11 -1474,7 +1476,11 @@@ static int resolve_missing_loose_ref(co
  }
  
  /* 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;
                bad_name = 1;
        }
        for (;;) {
 -              char path[PATH_MAX];
 +              const char *path;
                struct stat st;
                char *buf;
                int fd;
                        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
        }
  }
  
 +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));
@@@ -2292,7 -2273,7 +2291,7 @@@ static struct ref_lock *lock_ref_sha1_b
                                            const struct string_list *skip,
                                            unsigned int flags, int *type_p)
  {
 -      char *ref_file;
 +      const char *ref_file;
        const char *orig_refname = refname;
        struct ref_lock *lock;
        int last_errno = 0;
        int attempts_remaining = 3;
  
        lock = xcalloc(1, sizeof(struct ref_lock));
-       lock->lock_fd = -1;
  
        if (mustexist)
                resolve_flags |= RESOLVE_REF_READING;
        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:
                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)
                        /*
@@@ -2739,7 -2718,7 +2736,7 @@@ static int rename_tmp_log(const char *n
        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:
@@@ -2886,7 -2865,6 +2883,6 @@@ static int close_ref(struct ref_lock *l
  {
        if (close_lock_file(lock->lk))
                return -1;
-       lock->lock_fd = -1;
        return 0;
  }
  
@@@ -2894,7 -2872,6 +2890,6 @@@ static int commit_ref(struct ref_lock *
  {
        if (commit_lock_file(lock->lk))
                return -1;
-       lock->lock_fd = -1;
        return 0;
  }
  
@@@ -2925,15 -2902,11 +2920,15 @@@ static int copy_msg(char *buf, const ch
  }
  
  /* 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/") ||
@@@ -3004,22 -2977,18 +2999,22 @@@ static int log_ref_write_fd(int fd, con
        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)
        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/");
@@@ -3081,8 -3041,8 +3076,8 @@@ static int write_ref_sha1(struct ref_lo
                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);
@@@ -4119,9 -4079,9 +4114,9 @@@ int reflog_expire(const char *refname, 
                        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);