Merge branch 'jc/verify-loose-object-header'
authorJunio C Hamano <gitster@pobox.com>
Mon, 3 Oct 2016 20:30:36 +0000 (13:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 3 Oct 2016 20:30:36 +0000 (13:30 -0700)
Codepaths that read from an on-disk loose object were too loose in
validating what they are reading is a proper object file and
sometimes read past the data they read from the disk, which has
been corrected.  H/t to Gustavo Grieco for reporting.

* jc/verify-loose-object-header:
  unpack_sha1_header(): detect malformed object header
  streaming: make sure to notice corrupt object

1  2 
sha1_file.c
streaming.c

diff --combined sha1_file.c
@@@ -23,9 -23,6 +23,9 @@@
  #include "bulk-checkin.h"
  #include "streaming.h"
  #include "dir.h"
 +#include "mru.h"
 +#include "list.h"
 +#include "mergesort.h"
  
  #ifndef O_NOATIME
  #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
  static inline uintmax_t sz_fmt(size_t s) { return s; }
  
  const unsigned char null_sha1[20];
 +const struct object_id null_oid;
 +const struct object_id empty_tree_oid = {
 +      EMPTY_TREE_SHA1_BIN_LITERAL
 +};
 +const struct object_id empty_blob_oid = {
 +      EMPTY_BLOB_SHA1_BIN_LITERAL
 +};
  
  /*
   * This is meant to hold a *small* number of objects that you would
@@@ -68,6 -58,14 +68,6 @@@ static struct cached_object empty_tree 
        0
  };
  
 -/*
 - * A pointer to the last packed_git in which an object was found.
 - * When an object is sought, we look in this packfile first, because
 - * objects that are looked up at similar times are often in the same
 - * packfile as one another.
 - */
 -static struct packed_git *last_found_pack;
 -
  static struct cached_object *find_cached_object(const unsigned char *sha1)
  {
        int i;
@@@ -210,25 -208,44 +210,25 @@@ const char *sha1_file_name(const unsign
   * provided by the caller.  which should be "pack" or "idx".
   */
  static char *sha1_get_pack_name(const unsigned char *sha1,
 -                              char **name, char **base, const char *which)
 +                              struct strbuf *buf,
 +                              const char *which)
  {
 -      static const char hex[] = "0123456789abcdef";
 -      char *buf;
 -      int i;
 -
 -      if (!*base) {
 -              const char *sha1_file_directory = get_object_directory();
 -              int len = strlen(sha1_file_directory);
 -              *base = xmalloc(len + 60);
 -              sprintf(*base, "%s/pack/pack-1234567890123456789012345678901234567890.%s",
 -                      sha1_file_directory, which);
 -              *name = *base + len + 11;
 -      }
 -
 -      buf = *name;
 -
 -      for (i = 0; i < 20; i++) {
 -              unsigned int val = *sha1++;
 -              *buf++ = hex[val >> 4];
 -              *buf++ = hex[val & 0xf];
 -      }
 -
 -      return *base;
 +      strbuf_reset(buf);
 +      strbuf_addf(buf, "%s/pack/pack-%s.%s", get_object_directory(),
 +                  sha1_to_hex(sha1), which);
 +      return buf->buf;
  }
  
  char *sha1_pack_name(const unsigned char *sha1)
  {
 -      static char *name, *base;
 -
 -      return sha1_get_pack_name(sha1, &name, &base, "pack");
 +      static struct strbuf buf = STRBUF_INIT;
 +      return sha1_get_pack_name(sha1, &buf, "pack");
  }
  
  char *sha1_pack_index_name(const unsigned char *sha1)
  {
 -      static char *name, *base;
 -
 -      return sha1_get_pack_name(sha1, &name, &base, "idx");
 +      static struct strbuf buf = STRBUF_INIT;
 +      return sha1_get_pack_name(sha1, &buf, "idx");
  }
  
  struct alternate_object_database *alt_odb_list;
@@@ -254,7 -271,7 +254,7 @@@ static int link_alt_odb_entry(const cha
  {
        struct alternate_object_database *ent;
        struct alternate_object_database *alt;
 -      int pfxlen, entlen;
 +      size_t pfxlen, entlen;
        struct strbuf pathbuf = STRBUF_INIT;
  
        if (!is_absolute_path(entry) && relative_base) {
        while (pfxlen && pathbuf.buf[pfxlen-1] == '/')
                pfxlen -= 1;
  
 -      entlen = pfxlen + 43; /* '/' + 2 hex + '/' + 38 hex + NUL */
 -      ent = xmalloc(sizeof(*ent) + entlen);
 +      entlen = st_add(pfxlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
 +      ent = xmalloc(st_add(sizeof(*ent), entlen));
        memcpy(ent->base, pathbuf.buf, pfxlen);
        strbuf_release(&pathbuf);
  
                        return -1;
                }
        }
 -      if (!strcmp_icase(ent->base, normalized_objdir)) {
 +      if (!fspathcmp(ent->base, normalized_objdir)) {
                free(ent);
                return -1;
        }
@@@ -384,122 -401,13 +384,122 @@@ void read_info_alternates(const char * 
  void add_to_alternates_file(const char *reference)
  {
        struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 -      int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR);
 -      const char *alt = mkpath("%s\n", reference);
 -      write_or_die(fd, alt, strlen(alt));
 -      if (commit_lock_file(lock))
 -              die("could not close alternates file");
 -      if (alt_odb_tail)
 -              link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0);
 +      char *alts = git_pathdup("objects/info/alternates");
 +      FILE *in, *out;
 +
 +      hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);
 +      out = fdopen_lock_file(lock, "w");
 +      if (!out)
 +              die_errno("unable to fdopen alternates lockfile");
 +
 +      in = fopen(alts, "r");
 +      if (in) {
 +              struct strbuf line = STRBUF_INIT;
 +              int found = 0;
 +
 +              while (strbuf_getline(&line, in) != EOF) {
 +                      if (!strcmp(reference, line.buf)) {
 +                              found = 1;
 +                              break;
 +                      }
 +                      fprintf_or_die(out, "%s\n", line.buf);
 +              }
 +
 +              strbuf_release(&line);
 +              fclose(in);
 +
 +              if (found) {
 +                      rollback_lock_file(lock);
 +                      lock = NULL;
 +              }
 +      }
 +      else if (errno != ENOENT)
 +              die_errno("unable to read alternates file");
 +
 +      if (lock) {
 +              fprintf_or_die(out, "%s\n", reference);
 +              if (commit_lock_file(lock))
 +                      die_errno("unable to move new alternates file into place");
 +              if (alt_odb_tail)
 +                      link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
 +      }
 +      free(alts);
 +}
 +
 +/*
 + * Compute the exact path an alternate is at and returns it. In case of
 + * error NULL is returned and the human readable error is added to `err`
 + * `path` may be relative and should point to $GITDIR.
 + * `err` must not be null.
 + */
 +char *compute_alternate_path(const char *path, struct strbuf *err)
 +{
 +      char *ref_git = NULL;
 +      const char *repo, *ref_git_s;
 +      int seen_error = 0;
 +
 +      ref_git_s = real_path_if_valid(path);
 +      if (!ref_git_s) {
 +              seen_error = 1;
 +              strbuf_addf(err, _("path '%s' does not exist"), path);
 +              goto out;
 +      } else
 +              /*
 +               * Beware: read_gitfile(), real_path() and mkpath()
 +               * return static buffer
 +               */
 +              ref_git = xstrdup(ref_git_s);
 +
 +      repo = read_gitfile(ref_git);
 +      if (!repo)
 +              repo = read_gitfile(mkpath("%s/.git", ref_git));
 +      if (repo) {
 +              free(ref_git);
 +              ref_git = xstrdup(repo);
 +      }
 +
 +      if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) {
 +              char *ref_git_git = mkpathdup("%s/.git", ref_git);
 +              free(ref_git);
 +              ref_git = ref_git_git;
 +      } else if (!is_directory(mkpath("%s/objects", ref_git))) {
 +              struct strbuf sb = STRBUF_INIT;
 +              seen_error = 1;
 +              if (get_common_dir(&sb, ref_git)) {
 +                      strbuf_addf(err,
 +                                  _("reference repository '%s' as a linked "
 +                                    "checkout is not supported yet."),
 +                                  path);
 +                      goto out;
 +              }
 +
 +              strbuf_addf(err, _("reference repository '%s' is not a "
 +                                      "local repository."), path);
 +              goto out;
 +      }
 +
 +      if (!access(mkpath("%s/shallow", ref_git), F_OK)) {
 +              strbuf_addf(err, _("reference repository '%s' is shallow"),
 +                          path);
 +              seen_error = 1;
 +              goto out;
 +      }
 +
 +      if (!access(mkpath("%s/info/grafts", ref_git), F_OK)) {
 +              strbuf_addf(err,
 +                          _("reference repository '%s' is grafted"),
 +                          path);
 +              seen_error = 1;
 +              goto out;
 +      }
 +
 +out:
 +      if (seen_error) {
 +              free(ref_git);
 +              ref_git = NULL;
 +      }
 +
 +      return ref_git;
  }
  
  int foreach_alt_odb(alt_odb_fn fn, void *cb)
@@@ -599,9 -507,6 +599,9 @@@ static size_t peak_pack_mapped
  static size_t pack_mapped;
  struct packed_git *packed_git;
  
 +static struct mru packed_git_mru_storage;
 +struct mru *packed_git_mru = &packed_git_mru_storage;
 +
  void pack_report(void)
  {
        fprintf(stderr,
@@@ -733,15 -638,13 +733,15 @@@ static int check_packed_git_idx(const c
  int open_pack_index(struct packed_git *p)
  {
        char *idx_name;
 +      size_t len;
        int ret;
  
        if (p->index_data)
                return 0;
  
 -      idx_name = xstrdup(p->pack_name);
 -      strcpy(idx_name + strlen(idx_name) - strlen(".pack"), ".idx");
 +      if (!strip_suffix(p->pack_name, ".pack", &len))
 +              die("BUG: pack_name does not end in .pack");
 +      idx_name = xstrfmt("%.*s.idx", (int)len, p->pack_name);
        ret = check_packed_git_idx(idx_name, p);
        free(idx_name);
        return ret;
@@@ -850,37 -753,6 +850,37 @@@ void close_pack_windows(struct packed_g
        }
  }
  
 +static int close_pack_fd(struct packed_git *p)
 +{
 +      if (p->pack_fd < 0)
 +              return 0;
 +
 +      close(p->pack_fd);
 +      pack_open_fds--;
 +      p->pack_fd = -1;
 +
 +      return 1;
 +}
 +
 +static void close_pack(struct packed_git *p)
 +{
 +      close_pack_windows(p);
 +      close_pack_fd(p);
 +      close_pack_index(p);
 +}
 +
 +void close_all_packs(void)
 +{
 +      struct packed_git *p;
 +
 +      for (p = packed_git; p; p = p->next)
 +              if (p->do_not_close)
 +                      die("BUG: want to close pack marked 'do-not-close'");
 +              else
 +                      close_pack(p);
 +}
 +
 +
  /*
   * The LRU pack is the one with the oldest MRU window, preferring packs
   * with no used windows, or the oldest mtime if it has no windows allocated.
@@@ -948,8 -820,12 +948,8 @@@ static int close_one_pack(void
                find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
        }
  
 -      if (lru_p) {
 -              close(lru_p->pack_fd);
 -              pack_open_fds--;
 -              lru_p->pack_fd = -1;
 -              return 1;
 -      }
 +      if (lru_p)
 +              return close_pack_fd(lru_p);
  
        return 0;
  }
@@@ -971,6 -847,41 +971,6 @@@ void close_pack_index(struct packed_gi
        }
  }
  
 -/*
 - * This is used by git-repack in case a newly created pack happens to
 - * contain the same set of objects as an existing one.  In that case
 - * the resulting file might be different even if its name would be the
 - * same.  It is best to close any reference to the old pack before it is
 - * replaced on disk.  Of course no index pointers or windows for given pack
 - * must subsist at this point.  If ever objects from this pack are requested
 - * again, the new version of the pack will be reinitialized through
 - * reprepare_packed_git().
 - */
 -void free_pack_by_name(const char *pack_name)
 -{
 -      struct packed_git *p, **pp = &packed_git;
 -
 -      while (*pp) {
 -              p = *pp;
 -              if (strcmp(pack_name, p->pack_name) == 0) {
 -                      clear_delta_base_cache();
 -                      close_pack_windows(p);
 -                      if (p->pack_fd != -1) {
 -                              close(p->pack_fd);
 -                              pack_open_fds--;
 -                      }
 -                      close_pack_index(p);
 -                      free(p->bad_object_sha1);
 -                      *pp = p->next;
 -                      if (last_found_pack == p)
 -                              last_found_pack = NULL;
 -                      free(p);
 -                      return;
 -              }
 -              pp = &p->next;
 -      }
 -}
 -
  static unsigned int get_max_fd_limit(void)
  {
  #ifdef RLIMIT_NOFILE
@@@ -1093,7 -1004,11 +1093,7 @@@ static int open_packed_git(struct packe
  {
        if (!open_packed_git_1(p))
                return 0;
 -      if (p->pack_fd != -1) {
 -              close(p->pack_fd);
 -              pack_open_fds--;
 -              p->pack_fd = -1;
 -      }
 +      close_pack_fd(p);
        return -1;
  }
  
@@@ -1126,8 -1041,6 +1126,8 @@@ unsigned char *use_pack(struct packed_g
                die("packfile %s cannot be accessed", p->pack_name);
        if (offset > (p->pack_size - 20))
                die("offset beyond end of packfile (truncated pack?)");
 +      if (offset < 0)
 +              die(_("offset before end of packfile (broken .idx?)"));
  
        if (!win || !in_window(win, offset)) {
                if (win)
                                PROT_READ, MAP_PRIVATE,
                                p->pack_fd, win->offset);
                        if (win->base == MAP_FAILED)
 -                              die("packfile %s cannot be mapped: %s",
 -                                      p->pack_name,
 -                                      strerror(errno));
 +                              die_errno("packfile %s cannot be mapped",
 +                                        p->pack_name);
                        if (!win->offset && win->len == p->pack_size
 -                              && !p->do_not_close) {
 -                              close(p->pack_fd);
 -                              pack_open_fds--;
 -                              p->pack_fd = -1;
 -                      }
 +                              && !p->do_not_close)
 +                              close_pack_fd(p);
                        pack_mmap_calls++;
                        pack_open_windows++;
                        if (pack_mapped > peak_pack_mapped)
  
  static struct packed_git *alloc_packed_git(int extra)
  {
 -      struct packed_git *p = xmalloc(sizeof(*p) + extra);
 +      struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
        memset(p, 0, sizeof(*p));
        p->pack_fd = -1;
        return p;
@@@ -1196,12 -1113,11 +1196,12 @@@ static void try_to_free_pack_memory(siz
        release_pack_memory(size);
  }
  
 -struct packed_git *add_packed_git(const char *path, int path_len, int local)
 +struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
  {
        static int have_set_try_to_free_routine;
        struct stat st;
 -      struct packed_git *p = alloc_packed_git(path_len + 2);
 +      size_t alloc;
 +      struct packed_git *p;
  
        if (!have_set_try_to_free_routine) {
                have_set_try_to_free_routine = 1;
         * Make sure a corresponding .pack file exists and that
         * the index looks sane.
         */
 -      path_len -= strlen(".idx");
 -      if (path_len < 1) {
 -              free(p);
 +      if (!strip_suffix_mem(path, &path_len, ".idx"))
                return NULL;
 -      }
 +
 +      /*
 +       * ".pack" is long enough to hold any suffix we're adding (and
 +       * the use xsnprintf double-checks that)
 +       */
 +      alloc = st_add3(path_len, strlen(".pack"), 1);
 +      p = alloc_packed_git(alloc);
        memcpy(p->pack_name, path, path_len);
  
 -      strcpy(p->pack_name + path_len, ".keep");
 +      xsnprintf(p->pack_name + path_len, alloc - path_len, ".keep");
        if (!access(p->pack_name, F_OK))
                p->pack_keep = 1;
  
 -      strcpy(p->pack_name + path_len, ".pack");
 +      xsnprintf(p->pack_name + path_len, alloc - path_len, ".pack");
        if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {
                free(p);
                return NULL;
  struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path)
  {
        const char *path = sha1_pack_name(sha1);
 -      struct packed_git *p = alloc_packed_git(strlen(path) + 1);
 +      size_t alloc = st_add(strlen(path), 1);
 +      struct packed_git *p = alloc_packed_git(alloc);
  
 -      strcpy(p->pack_name, path);
 +      memcpy(p->pack_name, path, alloc); /* includes NUL */
        hashcpy(p->sha1, sha1);
        if (check_packed_git_idx(idx_path, p)) {
                free(p);
@@@ -1269,16 -1180,27 +1269,16 @@@ void install_packed_git(struct packed_g
        packed_git = pack;
  }
  
 -void (*report_garbage)(const char *desc, const char *path);
 +void (*report_garbage)(unsigned seen_bits, const char *path);
  
  static void report_helper(const struct string_list *list,
                          int seen_bits, int first, int last)
  {
 -      const char *msg;
 -      switch (seen_bits) {
 -      case 0:
 -              msg = "no corresponding .idx or .pack";
 -              break;
 -      case 1:
 -              msg = "no corresponding .idx";
 -              break;
 -      case 2:
 -              msg = "no corresponding .pack";
 -              break;
 -      default:
 +      if (seen_bits == (PACKDIR_FILE_PACK|PACKDIR_FILE_IDX))
                return;
 -      }
 +
        for (; first < last; first++)
 -              report_garbage(msg, list->items[first].string);
 +              report_garbage(seen_bits, list->items[first].string);
  }
  
  static void report_pack_garbage(struct string_list *list)
                if (baselen == -1) {
                        const char *dot = strrchr(path, '.');
                        if (!dot) {
 -                              report_garbage("garbage found", path);
 +                              report_garbage(PACKDIR_FILE_GARBAGE, path);
                                continue;
                        }
                        baselen = dot - path + 1;
@@@ -1328,8 -1250,8 +1328,8 @@@ static void prepare_packed_git_one(cha
        dir = opendir(path.buf);
        if (!dir) {
                if (errno != ENOENT)
 -                      error("unable to open object pack directory: %s: %s",
 -                            path.buf, strerror(errno));
 +                      error_errno("unable to open object pack directory: %s",
 +                                  path.buf);
                strbuf_release(&path);
                return;
        }
                    ends_with(de->d_name, ".keep"))
                        string_list_append(&garbage, path.buf);
                else
 -                      report_garbage("garbage found", path.buf);
 +                      report_garbage(PACKDIR_FILE_GARBAGE, path.buf);
        }
        closedir(dir);
        report_pack_garbage(&garbage);
        strbuf_release(&path);
  }
  
 +static void *get_next_packed_git(const void *p)
 +{
 +      return ((const struct packed_git *)p)->next;
 +}
 +
 +static void set_next_packed_git(void *p, void *next)
 +{
 +      ((struct packed_git *)p)->next = next;
 +}
 +
  static int sort_pack(const void *a_, const void *b_)
  {
 -      struct packed_git *a = *((struct packed_git **)a_);
 -      struct packed_git *b = *((struct packed_git **)b_);
 +      const struct packed_git *a = a_;
 +      const struct packed_git *b = b_;
        int st;
  
        /*
  
  static void rearrange_packed_git(void)
  {
 -      struct packed_git **ary, *p;
 -      int i, n;
 -
 -      for (n = 0, p = packed_git; p; p = p->next)
 -              n++;
 -      if (n < 2)
 -              return;
 -
 -      /* prepare an array of packed_git for easier sorting */
 -      ary = xcalloc(n, sizeof(struct packed_git *));
 -      for (n = 0, p = packed_git; p; p = p->next)
 -              ary[n++] = p;
 -
 -      qsort(ary, n, sizeof(struct packed_git *), sort_pack);
 +      packed_git = llist_mergesort(packed_git, get_next_packed_git,
 +                                   set_next_packed_git, sort_pack);
 +}
  
 -      /* link them back again */
 -      for (i = 0; i < n - 1; i++)
 -              ary[i]->next = ary[i + 1];
 -      ary[n - 1]->next = NULL;
 -      packed_git = ary[0];
 +static void prepare_packed_git_mru(void)
 +{
 +      struct packed_git *p;
  
 -      free(ary);
 +      mru_clear(packed_git_mru);
 +      for (p = packed_git; p; p = p->next)
 +              mru_append(packed_git_mru, p);
  }
  
  static int prepare_packed_git_run_once = 0;
@@@ -1449,7 -1372,6 +1449,7 @@@ void prepare_packed_git(void
                alt->name[-1] = '/';
        }
        rearrange_packed_git();
 +      prepare_packed_git_mru();
        prepare_packed_git_run_once = 1;
  }
  
@@@ -1464,12 -1386,10 +1464,12 @@@ static void mark_bad_packed_object(stru
  {
        unsigned i;
        for (i = 0; i < p->num_bad_objects; i++)
 -              if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
 +              if (!hashcmp(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i))
                        return;
 -      p->bad_object_sha1 = xrealloc(p->bad_object_sha1, 20 * (p->num_bad_objects + 1));
 -      hashcpy(p->bad_object_sha1 + 20 * p->num_bad_objects, sha1);
 +      p->bad_object_sha1 = xrealloc(p->bad_object_sha1,
 +                                    st_mult(GIT_SHA1_RAWSZ,
 +                                            st_add(p->num_bad_objects, 1)));
 +      hashcpy(p->bad_object_sha1 + GIT_SHA1_RAWSZ * p->num_bad_objects, sha1);
        p->num_bad_objects++;
  }
  
@@@ -1511,7 -1431,7 +1511,7 @@@ int check_sha1_signature(const unsigne
                return -1;
  
        /* Generate the header */
 -      hdrlen = sprintf(hdr, "%s %lu", typename(obj_type), size) + 1;
 +      hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(obj_type), size) + 1;
  
        /* Sha1.. */
        git_SHA1_Init(&c);
@@@ -1646,7 -1566,9 +1646,9 @@@ unsigned long unpack_object_header_buff
        return used;
  }
  
- int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)
+ static int unpack_sha1_short_header(git_zstream *stream,
+                                   unsigned char *map, unsigned long mapsize,
+                                   void *buffer, unsigned long bufsiz)
  {
        /* Get the data stream */
        memset(stream, 0, sizeof(*stream));
        return git_inflate(stream, 0);
  }
  
+ int unpack_sha1_header(git_zstream *stream,
+                      unsigned char *map, unsigned long mapsize,
+                      void *buffer, unsigned long bufsiz)
+ {
+       int status = unpack_sha1_short_header(stream, map, mapsize,
+                                             buffer, bufsiz);
+       if (status < Z_OK)
+               return status;
+       /* Make sure we have the terminating NUL */
+       if (!memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
+               return -1;
+       return 0;
+ }
  static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
                                        unsigned long mapsize, void *buffer,
                                        unsigned long bufsiz, struct strbuf *header)
  {
        int status;
  
-       status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+       status = unpack_sha1_short_header(stream, map, mapsize, buffer, bufsiz);
+       if (status < Z_OK)
+               return -1;
  
        /*
         * Check if entire header is unpacked in the first iteration.
@@@ -1756,6 -1696,8 +1776,8 @@@ static int parse_sha1_header_extended(c
         */
        for (;;) {
                char c = *hdr++;
+               if (!c)
+                       return -1;
                if (c == ' ')
                        break;
                type_len++;
                strbuf_add(oi->typename, type_buf, type_len);
        /*
         * Set type to 0 if its an unknown object and
 -       * we're obtaining the type using '--allow-unkown-type'
 +       * we're obtaining the type using '--allow-unknown-type'
         * option.
         */
        if ((flags & LOOKUP_UNKNOWN_OBJECT) && (type < 0))
@@@ -1995,7 -1937,7 +2017,7 @@@ static enum object_type packed_to_objec
                /* Push the object we're going to leave behind */
                if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
                        poi_stack_alloc = alloc_nr(poi_stack_nr);
 -                      poi_stack = xmalloc(sizeof(off_t)*poi_stack_alloc);
 +                      ALLOC_ARRAY(poi_stack, poi_stack_alloc);
                        memcpy(poi_stack, small_poi_stack, sizeof(off_t)*poi_stack_nr);
                } else {
                        ALLOC_GROW(poi_stack, poi_stack_nr+1, poi_stack_alloc);
@@@ -2147,142 -2089,136 +2169,142 @@@ static void *unpack_compressed_entry(st
        return buffer;
  }
  
 -#define MAX_DELTA_CACHE (256)
 -
 +static struct hashmap delta_base_cache;
  static size_t delta_base_cached;
  
 -static struct delta_base_cache_lru_list {
 -      struct delta_base_cache_lru_list *prev;
 -      struct delta_base_cache_lru_list *next;
 -} delta_base_cache_lru = { &delta_base_cache_lru, &delta_base_cache_lru };
 +static LIST_HEAD(delta_base_cache_lru);
  
 -static struct delta_base_cache_entry {
 -      struct delta_base_cache_lru_list lru;
 -      void *data;
 +struct delta_base_cache_key {
        struct packed_git *p;
        off_t base_offset;
 +};
 +
 +struct delta_base_cache_entry {
 +      struct hashmap hash;
 +      struct delta_base_cache_key key;
 +      struct list_head lru;
 +      void *data;
        unsigned long size;
        enum object_type type;
 -} delta_base_cache[MAX_DELTA_CACHE];
 +};
  
 -static unsigned long pack_entry_hash(struct packed_git *p, off_t base_offset)
 +static unsigned int pack_entry_hash(struct packed_git *p, off_t base_offset)
  {
 -      unsigned long hash;
 +      unsigned int hash;
  
 -      hash = (unsigned long)p + (unsigned long)base_offset;
 +      hash = (unsigned int)(intptr_t)p + (unsigned int)base_offset;
        hash += (hash >> 8) + (hash >> 16);
 -      return hash % MAX_DELTA_CACHE;
 +      return hash;
  }
  
  static struct delta_base_cache_entry *
  get_delta_base_cache_entry(struct packed_git *p, off_t base_offset)
  {
 -      unsigned long hash = pack_entry_hash(p, base_offset);
 -      return delta_base_cache + hash;
 +      struct hashmap_entry entry;
 +      struct delta_base_cache_key key;
 +
 +      if (!delta_base_cache.cmpfn)
 +              return NULL;
 +
 +      hashmap_entry_init(&entry, pack_entry_hash(p, base_offset));
 +      key.p = p;
 +      key.base_offset = base_offset;
 +      return hashmap_get(&delta_base_cache, &entry, &key);
 +}
 +
 +static int delta_base_cache_key_eq(const struct delta_base_cache_key *a,
 +                                 const struct delta_base_cache_key *b)
 +{
 +      return a->p == b->p && a->base_offset == b->base_offset;
  }
  
 -static int eq_delta_base_cache_entry(struct delta_base_cache_entry *ent,
 -                                   struct packed_git *p, off_t base_offset)
 +static int delta_base_cache_hash_cmp(const void *va, const void *vb,
 +                                   const void *vkey)
  {
 -      return (ent->data && ent->p == p && ent->base_offset == base_offset);
 +      const struct delta_base_cache_entry *a = va, *b = vb;
 +      const struct delta_base_cache_key *key = vkey;
 +      if (key)
 +              return !delta_base_cache_key_eq(&a->key, key);
 +      else
 +              return !delta_base_cache_key_eq(&a->key, &b->key);
  }
  
  static int in_delta_base_cache(struct packed_git *p, off_t base_offset)
  {
 -      struct delta_base_cache_entry *ent;
 -      ent = get_delta_base_cache_entry(p, base_offset);
 -      return eq_delta_base_cache_entry(ent, p, base_offset);
 +      return !!get_delta_base_cache_entry(p, base_offset);
  }
  
 -static void clear_delta_base_cache_entry(struct delta_base_cache_entry *ent)
 +/*
 + * Remove the entry from the cache, but do _not_ free the associated
 + * entry data. The caller takes ownership of the "data" buffer, and
 + * should copy out any fields it wants before detaching.
 + */
 +static void detach_delta_base_cache_entry(struct delta_base_cache_entry *ent)
  {
 -      ent->data = NULL;
 -      ent->lru.next->prev = ent->lru.prev;
 -      ent->lru.prev->next = ent->lru.next;
 +      hashmap_remove(&delta_base_cache, ent, &ent->key);
 +      list_del(&ent->lru);
        delta_base_cached -= ent->size;
 +      free(ent);
  }
  
  static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
 -      unsigned long *base_size, enum object_type *type, int keep_cache)
 +      unsigned long *base_size, enum object_type *type)
  {
        struct delta_base_cache_entry *ent;
 -      void *ret;
  
        ent = get_delta_base_cache_entry(p, base_offset);
 -
 -      if (!eq_delta_base_cache_entry(ent, p, base_offset))
 +      if (!ent)
                return unpack_entry(p, base_offset, type, base_size);
  
 -      ret = ent->data;
 -
 -      if (!keep_cache)
 -              clear_delta_base_cache_entry(ent);
 -      else
 -              ret = xmemdupz(ent->data, ent->size);
        *type = ent->type;
        *base_size = ent->size;
 -      return ret;
 +      return xmemdupz(ent->data, ent->size);
  }
  
  static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
  {
 -      if (ent->data) {
 -              free(ent->data);
 -              ent->data = NULL;
 -              ent->lru.next->prev = ent->lru.prev;
 -              ent->lru.prev->next = ent->lru.next;
 -              delta_base_cached -= ent->size;
 -      }
 +      free(ent->data);
 +      detach_delta_base_cache_entry(ent);
  }
  
  void clear_delta_base_cache(void)
  {
 -      unsigned long p;
 -      for (p = 0; p < MAX_DELTA_CACHE; p++)
 -              release_delta_base_cache(&delta_base_cache[p]);
 +      struct hashmap_iter iter;
 +      struct delta_base_cache_entry *entry;
 +      for (entry = hashmap_iter_first(&delta_base_cache, &iter);
 +           entry;
 +           entry = hashmap_iter_next(&iter)) {
 +              release_delta_base_cache(entry);
 +      }
  }
  
  static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
        void *base, unsigned long base_size, enum object_type type)
  {
 -      unsigned long hash = pack_entry_hash(p, base_offset);
 -      struct delta_base_cache_entry *ent = delta_base_cache + hash;
 -      struct delta_base_cache_lru_list *lru;
 +      struct delta_base_cache_entry *ent = xmalloc(sizeof(*ent));
 +      struct list_head *lru, *tmp;
  
 -      release_delta_base_cache(ent);
        delta_base_cached += base_size;
  
 -      for (lru = delta_base_cache_lru.next;
 -           delta_base_cached > delta_base_cache_limit
 -           && lru != &delta_base_cache_lru;
 -           lru = lru->next) {
 -              struct delta_base_cache_entry *f = (void *)lru;
 -              if (f->type == OBJ_BLOB)
 -                      release_delta_base_cache(f);
 -      }
 -      for (lru = delta_base_cache_lru.next;
 -           delta_base_cached > delta_base_cache_limit
 -           && lru != &delta_base_cache_lru;
 -           lru = lru->next) {
 -              struct delta_base_cache_entry *f = (void *)lru;
 +      list_for_each_safe(lru, tmp, &delta_base_cache_lru) {
 +              struct delta_base_cache_entry *f =
 +                      list_entry(lru, struct delta_base_cache_entry, lru);
 +              if (delta_base_cached <= delta_base_cache_limit)
 +                      break;
                release_delta_base_cache(f);
        }
  
 -      ent->p = p;
 -      ent->base_offset = base_offset;
 +      ent->key.p = p;
 +      ent->key.base_offset = base_offset;
        ent->type = type;
        ent->data = base;
        ent->size = base_size;
 -      ent->lru.next = &delta_base_cache_lru;
 -      ent->lru.prev = delta_base_cache_lru.prev;
 -      delta_base_cache_lru.prev->next = &ent->lru;
 -      delta_base_cache_lru.prev = &ent->lru;
 +      list_add_tail(&ent->lru, &delta_base_cache_lru);
 +
 +      if (!delta_base_cache.cmpfn)
 +              hashmap_init(&delta_base_cache, delta_base_cache_hash_cmp, 0);
 +      hashmap_entry_init(ent, pack_entry_hash(p, base_offset));
 +      hashmap_add(&delta_base_cache, ent);
  }
  
  static void *read_object(const unsigned char *sha1, enum object_type *type,
@@@ -2326,18 -2262,18 +2348,18 @@@ void *unpack_entry(struct packed_git *p
                struct delta_base_cache_entry *ent;
  
                ent = get_delta_base_cache_entry(p, curpos);
 -              if (eq_delta_base_cache_entry(ent, p, curpos)) {
 +              if (ent) {
                        type = ent->type;
                        data = ent->data;
                        size = ent->size;
 -                      clear_delta_base_cache_entry(ent);
 +                      detach_delta_base_cache_entry(ent);
                        base_from_cache = 1;
                        break;
                }
  
                if (do_check_packed_object_crc && p->index_version > 1) {
                        struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
 -                      unsigned long len = revidx[1].offset - obj_offset;
 +                      off_t len = revidx[1].offset - obj_offset;
                        if (check_pack_crc(p, &w_curs, obj_offset, len, revidx->nr)) {
                                const unsigned char *sha1 =
                                        nth_packed_object_sha1(p, revidx->nr);
                if (delta_stack_nr >= delta_stack_alloc
                    && delta_stack == small_delta_stack) {
                        delta_stack_alloc = alloc_nr(delta_stack_nr);
 -                      delta_stack = xmalloc(sizeof(*delta_stack)*delta_stack_alloc);
 +                      ALLOC_ARRAY(delta_stack, delta_stack_alloc);
                        memcpy(delta_stack, small_delta_stack,
                               sizeof(*delta_stack)*delta_stack_nr);
                } else {
        case OBJ_OFS_DELTA:
        case OBJ_REF_DELTA:
                if (data)
 -                      die("BUG in unpack_entry: left loop at a valid delta");
 +                      die("BUG: unpack_entry: left loop at a valid delta");
                break;
        case OBJ_COMMIT:
        case OBJ_TREE:
@@@ -2505,20 -2441,6 +2527,20 @@@ const unsigned char *nth_packed_object_
        }
  }
  
 +void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 +{
 +      const unsigned char *ptr = vptr;
 +      const unsigned char *start = p->index_data;
 +      const unsigned char *end = start + p->index_size;
 +      if (ptr < start)
 +              die(_("offset before start of pack index for %s (corrupt index?)"),
 +                  p->pack_name);
 +      /* No need to check for underflow; .idx files must be at least 8 bytes */
 +      if (ptr >= end - 8)
 +              die(_("offset beyond end of pack index for %s (truncated index?)"),
 +                  p->pack_name);
 +}
 +
  off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
  {
        const unsigned char *index = p->index_data;
                if (!(off & 0x80000000))
                        return off;
                index += p->num_objects * 4 + (off & 0x7fffffff) * 8;
 +              check_pack_index_ptr(p, index);
                return (((uint64_t)ntohl(*((uint32_t *)(index + 0)))) << 32) |
                                   ntohl(*((uint32_t *)(index + 4)));
        }
@@@ -2660,15 -2581,21 +2682,15 @@@ static int fill_pack_entry(const unsign
   */
  static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
  {
 -      struct packed_git *p;
 +      struct mru_entry *p;
  
        prepare_packed_git();
        if (!packed_git)
                return 0;
  
 -      if (last_found_pack && fill_pack_entry(sha1, e, last_found_pack))
 -              return 1;
 -
 -      for (p = packed_git; p; p = p->next) {
 -              if (p == last_found_pack)
 -                      continue; /* we already checked this one */
 -
 -              if (fill_pack_entry(sha1, e, p)) {
 -                      last_found_pack = p;
 +      for (p = packed_git_mru->head; p; p = p->next) {
 +              if (fill_pack_entry(sha1, e, p->item)) {
 +                      mru_mark(packed_git_mru, p);
                        return 1;
                }
        }
@@@ -2835,7 -2762,7 +2857,7 @@@ static void *read_packed_sha1(const uns
  
        if (!find_pack_entry(sha1, &e))
                return NULL;
 -      data = cache_or_unpack_entry(e.p, e.offset, size, type, 1);
 +      data = cache_or_unpack_entry(e.p, e.offset, size, type);
        if (!data) {
                /*
                 * We're probably in deep shit, but let's try to fetch
@@@ -2992,7 -2919,7 +3014,7 @@@ static void write_sha1_file_prepare(con
        git_SHA_CTX c;
  
        /* Generate the header */
 -      *hdrlen = sprintf(hdr, "%s %lu", type, len)+1;
 +      *hdrlen = xsnprintf(hdr, *hdrlen, "%s %lu", type, len)+1;
  
        /* Sha1.. */
        git_SHA1_Init(&c);
  
  /*
   * Move the just written object into its final resting place.
 - * NEEDSWORK: this should be renamed to finalize_temp_file() as
 - * "moving" is only a part of what it does, when no patch between
 - * master to pu changes the call sites of this function.
   */
 -int move_temp_to_file(const char *tmpfile, const char *filename)
 +int finalize_object_file(const char *tmpfile, const char *filename)
  {
        int ret = 0;
  
        unlink_or_warn(tmpfile);
        if (ret) {
                if (ret != EEXIST) {
 -                      return error("unable to write sha1 filename %s: %s", filename, strerror(ret));
 +                      return error_errno("unable to write sha1 filename %s", filename);
                }
                /* FIXME!!! Collision check here ? */
        }
@@@ -3047,7 -2977,7 +3069,7 @@@ out
  static int write_buffer(int fd, const void *buf, size_t len)
  {
        if (write_in_full(fd, buf, len) < 0)
 -              return error("file write error (%s)", strerror(errno));
 +              return error_errno("file write error");
        return 0;
  }
  
@@@ -3055,7 -2985,7 +3077,7 @@@ int hash_sha1_file(const void *buf, uns
                     unsigned char *sha1)
  {
        char hdr[32];
 -      int hdrlen;
 +      int hdrlen = sizeof(hdr);
        write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
        return 0;
  }
@@@ -3085,31 -3015,29 +3107,31 @@@ static inline int directory_size(const 
   * We want to avoid cross-directory filename renames, because those
   * can have problems on various filesystems (FAT, NFS, Coda).
   */
 -static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
 +static int create_tmpfile(struct strbuf *tmp, const char *filename)
  {
        int fd, dirlen = directory_size(filename);
  
 -      if (dirlen + 20 > bufsiz) {
 -              errno = ENAMETOOLONG;
 -              return -1;
 -      }
 -      memcpy(buffer, filename, dirlen);
 -      strcpy(buffer + dirlen, "tmp_obj_XXXXXX");
 -      fd = git_mkstemp_mode(buffer, 0444);
 +      strbuf_reset(tmp);
 +      strbuf_add(tmp, filename, dirlen);
 +      strbuf_addstr(tmp, "tmp_obj_XXXXXX");
 +      fd = git_mkstemp_mode(tmp->buf, 0444);
        if (fd < 0 && dirlen && errno == ENOENT) {
 -              /* Make sure the directory exists */
 -              memcpy(buffer, filename, dirlen);
 -              buffer[dirlen-1] = 0;
 -              if (mkdir(buffer, 0777) && errno != EEXIST)
 +              /*
 +               * Make sure the directory exists; note that the contents
 +               * of the buffer are undefined after mkstemp returns an
 +               * error, so we have to rewrite the whole buffer from
 +               * scratch.
 +               */
 +              strbuf_reset(tmp);
 +              strbuf_add(tmp, filename, dirlen - 1);
 +              if (mkdir(tmp->buf, 0777) && errno != EEXIST)
                        return -1;
 -              if (adjust_shared_perm(buffer))
 +              if (adjust_shared_perm(tmp->buf))
                        return -1;
  
                /* Try again */
 -              strcpy(buffer + dirlen - 1, "/tmp_obj_XXXXXX");
 -              fd = git_mkstemp_mode(buffer, 0444);
 +              strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
 +              fd = git_mkstemp_mode(tmp->buf, 0444);
        }
        return fd;
  }
@@@ -3122,15 -3050,15 +3144,15 @@@ static int write_loose_object(const uns
        git_zstream stream;
        git_SHA_CTX c;
        unsigned char parano_sha1[20];
 -      static char tmp_file[PATH_MAX];
 +      static struct strbuf tmp_file = STRBUF_INIT;
        const char *filename = sha1_file_name(sha1);
  
 -      fd = create_tmpfile(tmp_file, sizeof(tmp_file), filename);
 +      fd = create_tmpfile(&tmp_file, filename);
        if (fd < 0) {
                if (errno == EACCES)
                        return error("insufficient permission for adding an object to repository database %s", get_object_directory());
                else
 -                      return error("unable to create temporary file: %s", strerror(errno));
 +                      return error_errno("unable to create temporary file");
        }
  
        /* Set it up */
                struct utimbuf utb;
                utb.actime = mtime;
                utb.modtime = mtime;
 -              if (utime(tmp_file, &utb) < 0)
 -                      warning("failed utime() on %s: %s",
 -                              tmp_file, strerror(errno));
 +              if (utime(tmp_file.buf, &utb) < 0)
 +                      warning_errno("failed utime() on %s", tmp_file.buf);
        }
  
 -      return move_temp_to_file(tmp_file, filename);
 +      return finalize_object_file(tmp_file.buf, filename);
  }
  
  static int freshen_loose_object(const unsigned char *sha1)
@@@ -3202,7 -3131,7 +3224,7 @@@ static int freshen_packed_object(const 
  int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1)
  {
        char hdr[32];
 -      int hdrlen;
 +      int hdrlen = sizeof(hdr);
  
        /* Normally if we have it in the pack then we do not bother writing
         * it out into .git/objects/??/?{38} file.
@@@ -3220,8 -3149,7 +3242,8 @@@ int hash_sha1_file_literally(const voi
        int hdrlen, status = 0;
  
        /* type string, SP, %lu of the length plus NUL must fit this */
 -      header = xmalloc(strlen(type) + 32);
 +      hdrlen = strlen(type) + 32;
 +      header = xmalloc(hdrlen);
        write_sha1_file_prepare(buf, len, type, sha1, header, &hdrlen);
  
        if (!(flags & HASH_WRITE_OBJECT))
@@@ -3249,7 -3177,7 +3271,7 @@@ int force_object_loose(const unsigned c
        buf = read_packed_sha1(sha1, &type, &len);
        if (!buf)
                return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
 -      hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;
 +      hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
        ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
        free(buf);
  
@@@ -3284,11 -3212,6 +3306,11 @@@ int has_sha1_file_with_flags(const unsi
        return find_pack_entry(sha1, &e);
  }
  
 +int has_object_file(const struct object_id *oid)
 +{
 +      return has_sha1_file(oid->hash);
 +}
 +
  static void check_tree(const void *buf, size_t size)
  {
        struct tree_desc desc;
@@@ -3408,7 -3331,7 +3430,7 @@@ static int index_core(unsigned char *sh
                if (size == read_in_full(fd, buf, size))
                        ret = index_mem(sha1, buf, size, type, path, flags);
                else
 -                      ret = error("short read %s", strerror(errno));
 +                      ret = error_errno("short read");
                free(buf);
        } else {
                void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
@@@ -3473,14 -3396,18 +3495,14 @@@ int index_path(unsigned char *sha1, con
        case S_IFREG:
                fd = open(path, O_RDONLY);
                if (fd < 0)
 -                      return error("open(\"%s\"): %s", path,
 -                                   strerror(errno));
 +                      return error_errno("open(\"%s\")", path);
                if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
                        return error("%s: failed to insert into database",
                                     path);
                break;
        case S_IFLNK:
 -              if (strbuf_readlink(&sb, path, st->st_size)) {
 -                      char *errstr = strerror(errno);
 -                      return error("readlink(\"%s\"): %s", path,
 -                                   errstr);
 -              }
 +              if (strbuf_readlink(&sb, path, st->st_size))
 +                      return error_errno("readlink(\"%s\")", path);
                if (!(flags & HASH_WRITE_OBJECT))
                        hash_sha1_file(sb.buf, sb.len, blob_type, sha1);
                else if (write_sha1_file(sb.buf, sb.len, blob_type, sha1))
@@@ -3536,7 -3463,7 +3558,7 @@@ static int for_each_file_in_obj_subdir(
        if (!dir) {
                if (errno == ENOENT)
                        return 0;
 -              return error("unable to open %s: %s", path->buf, strerror(errno));
 +              return error_errno("unable to open %s", path->buf);
        }
  
        while ((de = readdir(dir))) {
                                break;
                }
        }
 -      strbuf_setlen(path, baselen);
 +      closedir(dir);
  
 +      strbuf_setlen(path, baselen);
        if (!r && subdir_cb)
                r = subdir_cb(subdir_nr, path->buf, data);
  
 -      closedir(dir);
        return r;
  }
  
diff --combined streaming.c
@@@ -337,17 -337,17 +337,17 @@@ static open_method_decl(loose
        st->u.loose.mapped = map_sha1_file(sha1, &st->u.loose.mapsize);
        if (!st->u.loose.mapped)
                return -1;
-       if (unpack_sha1_header(&st->z,
-                              st->u.loose.mapped,
-                              st->u.loose.mapsize,
-                              st->u.loose.hdr,
-                              sizeof(st->u.loose.hdr)) < 0) {
+       if ((unpack_sha1_header(&st->z,
+                               st->u.loose.mapped,
+                               st->u.loose.mapsize,
+                               st->u.loose.hdr,
+                               sizeof(st->u.loose.hdr)) < 0) ||
+           (parse_sha1_header(st->u.loose.hdr, &st->size) < 0)) {
                git_inflate_end(&st->z);
                munmap(st->u.loose.mapped, st->u.loose.mapsize);
                return -1;
        }
  
-       parse_sha1_header(st->u.loose.hdr, &st->size);
        st->u.loose.hdr_used = strlen(st->u.loose.hdr) + 1;
        st->u.loose.hdr_avail = st->z.total_out;
        st->z_state = z_used;
@@@ -497,7 -497,7 +497,7 @@@ static open_method_decl(incore
   * Users of streaming interface
   ****************************************************************/
  
 -int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *filter,
 +int stream_blob_to_fd(int fd, const struct object_id *oid, struct stream_filter *filter,
                      int can_seek)
  {
        struct git_istream *st;
        ssize_t kept = 0;
        int result = -1;
  
 -      st = open_istream(sha1, &type, &sz, filter);
 +      st = open_istream(oid->hash, &type, &sz, filter);
        if (!st) {
                if (filter)
                        free_stream_filter(filter);