Merge branch 'jk/reading-packed-refs'
authorJunio C Hamano <gitster@pobox.com>
Mon, 11 May 2015 21:23:42 +0000 (14:23 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 11 May 2015 21:23:42 +0000 (14:23 -0700)
An earlier rewrite to use strbuf_getwholeline() instead of fgets(3)
to read packed-refs file revealed that the former is unacceptably
inefficient.

* jk/reading-packed-refs:
t1430: add another refs-escape test
read_packed_refs: avoid double-checking sane refs
strbuf_getwholeline: use getdelim if it is available
strbuf_getwholeline: avoid calling strbuf_grow
strbuf_addch: avoid calling strbuf_grow
config: use getc_unlocked when reading from file
strbuf_getwholeline: use getc_unlocked
git-compat-util: add fallbacks for unlocked stdio
strbuf_getwholeline: use getc macro

1  2 
config.c
refs.c
diff --combined config.c
index c4424c01388496b5995e19f9601f0c87b9fdd3c0,8b297fc4e5e05c2ad817f41399827d603e94d844..6d0098fc80df01ce9df4701119557472430b0619
+++ b/config.c
@@@ -12,7 -12,6 +12,7 @@@
  #include "quote.h"
  #include "hashmap.h"
  #include "string-list.h"
 +#include "utf8.h"
  
  struct config_source {
        struct config_source *prev;
@@@ -50,7 -49,7 +50,7 @@@ static struct config_set the_config_set
  
  static int config_file_fgetc(struct config_source *conf)
  {
-       return fgetc(conf->u.file);
+       return getc_unlocked(conf->u.file);
  }
  
  static int config_file_ungetc(int c, struct config_source *conf)
@@@ -418,7 -417,8 +418,7 @@@ static int git_parse_source(config_fn_
        struct strbuf *var = &cf->var;
  
        /* U+FEFF Byte Order Mark in UTF8 */
 -      static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf";
 -      const unsigned char *bomptr = utf8_bom;
 +      const char *bomptr = utf8_bom;
  
        for (;;) {
                int c = get_next_char();
                        /* We are at the file beginning; skip UTF8-encoded BOM
                         * if present. Sane editors won't put this in on their
                         * own, but e.g. Windows Notepad will do it happily. */
 -                      if ((unsigned char) c == *bomptr) {
 +                      if (c == (*bomptr & 0377)) {
                                bomptr++;
                                continue;
                        } else {
@@@ -1088,7 -1088,9 +1088,9 @@@ int git_config_from_file(config_fn_t fn
  
        f = fopen(filename, "r");
        if (f) {
+               flockfile(f);
                ret = do_config_from_file(fn, filename, filename, f, data);
+               funlockfile(f);
                fclose(f);
        }
        return ret;
diff --combined refs.c
index 0312f052575046f0dd539b19897b61eddc2862e0,f36ea75c879bd9d8dae6784c19e85f4ff9848a11..81a455b807e4ef7f79e3cdb252eedfccfa3c7d8a
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -344,8 -344,6 +344,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);
@@@ -1178,6 -1176,8 +1176,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,6 -1323,8 +1323,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;
                        }
@@@ -1382,7 -1384,7 +1384,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;
@@@ -1475,11 -1477,7 +1477,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));
@@@ -2290,7 -2276,7 +2292,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;
        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:
@@@ -2737,7 -2723,7 +2739,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:
@@@ -2923,15 -2909,11 +2925,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/") ||
@@@ -3002,22 -2984,18 +3004,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/");