Merge branch 'jk/server-info-rabbit-hole'
authorJunio C Hamano <gitster@pobox.com>
Thu, 25 Apr 2019 07:41:13 +0000 (16:41 +0900)
committerJunio C Hamano <gitster@pobox.com>
Thu, 25 Apr 2019 07:41:13 +0000 (16:41 +0900)
Code clean-up around a much-less-important-than-it-used-to-be
update_server_info() funtion.

* jk/server-info-rabbit-hole:
update_info_refs(): drop unused force parameter
server-info: drop objdirlen pointer arithmetic
server-info: drop nr_alloc struct member
server-info: use strbuf to read old info/packs file
server-info: simplify cleanup in parse_pack_def()
server-info: fix blind pointer arithmetic
http: simplify parsing of remote objects/info/packs
packfile: fix pack basename computation
midx: check both pack and index names for containment
t5319: drop useless --buffer from cat-file
t5319: fix bogus cat-file argument
pack-revindex: open index if necessary
packfile.h: drop extern from function declarations

1  2 
http.c
midx.c
packfile.c
packfile.h
t/t5570-git-daemon.sh
diff --combined http.c
index 89fcd36a80821464dc15c10cfe9c8a424256aeba,2ef47bc7798d87e9c4f2c6ff66c99e3db073048f..b5833550ca38c922fb4fe00d35b1d09de3e4196d
--- 1/http.c
--- 2/http.c
+++ b/http.c
@@@ -1544,8 -1544,7 +1544,8 @@@ char *get_remote_object_url(const char 
        return strbuf_detach(&buf, NULL);
  }
  
 -static int handle_curl_result(struct slot_results *results)
 +void normalize_curl_result(CURLcode *result, long http_code,
 +                         char *errorstr, size_t errorlen)
  {
        /*
         * If we see a failing http code with CURLE_OK, we have turned off
         * Likewise, if we see a redirect (30x code), that means we turned off
         * redirect-following, and we should treat the result as an error.
         */
 -      if (results->curl_result == CURLE_OK &&
 -          results->http_code >= 300) {
 -              results->curl_result = CURLE_HTTP_RETURNED_ERROR;
 +      if (*result == CURLE_OK && http_code >= 300) {
 +              *result = CURLE_HTTP_RETURNED_ERROR;
                /*
                 * Normally curl will already have put the "reason phrase"
                 * from the server into curl_errorstr; unfortunately without
                 * FAILONERROR it is lost, so we can give only the numeric
                 * status code.
                 */
 -              xsnprintf(curl_errorstr, sizeof(curl_errorstr),
 +              xsnprintf(errorstr, errorlen,
                          "The requested URL returned error: %ld",
 -                        results->http_code);
 +                        http_code);
        }
 +}
 +
 +static int handle_curl_result(struct slot_results *results)
 +{
 +      normalize_curl_result(&results->curl_result, results->http_code,
 +                            curl_errorstr, sizeof(curl_errorstr));
  
        if (results->curl_result == CURLE_OK) {
                credential_approve(&http_auth);
@@@ -2153,11 -2147,11 +2153,11 @@@ add_pack
  int http_get_info_packs(const char *base_url, struct packed_git **packs_head)
  {
        struct http_get_options options = {0};
-       int ret = 0, i = 0;
-       char *url, *data;
+       int ret = 0;
+       char *url;
+       const char *data;
        struct strbuf buf = STRBUF_INIT;
-       unsigned char hash[GIT_MAX_RAWSZ];
-       const unsigned hexsz = the_hash_algo->hexsz;
+       struct object_id oid;
  
        end_url_with_slash(&buf, base_url);
        strbuf_addstr(&buf, "objects/info/packs");
                goto cleanup;
  
        data = buf.buf;
-       while (i < buf.len) {
-               switch (data[i]) {
-               case 'P':
-                       i++;
-                       if (i + hexsz + 12 <= buf.len &&
-                           starts_with(data + i, " pack-") &&
-                           starts_with(data + i + hexsz + 6, ".pack\n")) {
-                               get_sha1_hex(data + i + 6, hash);
-                               fetch_and_setup_pack_index(packs_head, hash,
-                                                     base_url);
-                               i += hexsz + 11;
-                               break;
-                       }
-               default:
-                       while (i < buf.len && data[i] != '\n')
-                               i++;
+       while (*data) {
+               if (skip_prefix(data, "P pack-", &data) &&
+                   !parse_oid_hex(data, &oid, &data) &&
+                   skip_prefix(data, ".pack", &data) &&
+                   (*data == '\n' || *data == '\0')) {
+                       fetch_and_setup_pack_index(packs_head, oid.hash, base_url);
+               } else {
+                       data = strchrnul(data, '\n');
                }
-               i++;
+               if (*data)
+                       data++; /* skip past newline */
        }
  
  cleanup:
diff --combined midx.c
index f1137eb6ee7bb2d16062399965671cfda0e95e52,0ceca1938f44117f2655b0012a9e0ca6906ce9df..d5d2e9522fe39dc1032df4dcdea34e0eb3c741fb
--- 1/midx.c
--- 2/midx.c
+++ b/midx.c
@@@ -8,7 -8,6 +8,7 @@@
  #include "sha1-lookup.h"
  #include "midx.h"
  #include "progress.h"
 +#include "trace2.h"
  
  #define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
  #define MIDX_VERSION 1
@@@ -71,7 -70,7 +71,7 @@@ struct multi_pack_index *load_multi_pac
  
        midx_map = xmmap(NULL, midx_size, PROT_READ, MAP_PRIVATE, fd, 0);
  
 -      FLEX_ALLOC_MEM(m, object_dir, object_dir, strlen(object_dir));
 +      FLEX_ALLOC_STR(m, object_dir, object_dir);
        m->fd = fd;
        m->data = midx_map;
        m->data_len = midx_size;
                              m->pack_names[i]);
        }
  
 +      trace2_data_intmax("midx", the_repository, "load/num_packs", m->num_packs);
 +      trace2_data_intmax("midx", the_repository, "load/num_objects", m->num_objects);
 +
        return m;
  
  cleanup_fail:
@@@ -311,7 -307,39 +311,39 @@@ int fill_midx_entry(const struct object
        return nth_midxed_pack_entry(m, e, pos);
  }
  
- int midx_contains_pack(struct multi_pack_index *m, const char *idx_name)
+ /* Match "foo.idx" against either "foo.pack" _or_ "foo.idx". */
+ static int cmp_idx_or_pack_name(const char *idx_or_pack_name,
+                               const char *idx_name)
+ {
+       /* Skip past any initial matching prefix. */
+       while (*idx_name && *idx_name == *idx_or_pack_name) {
+               idx_name++;
+               idx_or_pack_name++;
+       }
+       /*
+        * If we didn't match completely, we may have matched "pack-1234." and
+        * be left with "idx" and "pack" respectively, which is also OK. We do
+        * not have to check for "idx" and "idx", because that would have been
+        * a complete match (and in that case these strcmps will be false, but
+        * we'll correctly return 0 from the final strcmp() below.
+        *
+        * Technically this matches "fooidx" and "foopack", but we'd never have
+        * such names in the first place.
+        */
+       if (!strcmp(idx_name, "idx") && !strcmp(idx_or_pack_name, "pack"))
+               return 0;
+       /*
+        * This not only checks for a complete match, but also orders based on
+        * the first non-identical character, which means our ordering will
+        * match a raw strcmp(). That makes it OK to use this to binary search
+        * a naively-sorted list.
+        */
+       return strcmp(idx_or_pack_name, idx_name);
+ }
+ int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name)
  {
        uint32_t first = 0, last = m->num_packs;
  
                int cmp;
  
                current = m->pack_names[mid];
-               cmp = strcmp(idx_name, current);
+               cmp = cmp_idx_or_pack_name(idx_or_pack_name, current);
                if (!cmp)
                        return 1;
                if (cmp > 0) {
@@@ -962,35 -990,8 +994,35 @@@ static void midx_report(const char *fmt
        va_end(ap);
  }
  
 +struct pair_pos_vs_id
 +{
 +      uint32_t pos;
 +      uint32_t pack_int_id;
 +};
 +
 +static int compare_pair_pos_vs_id(const void *_a, const void *_b)
 +{
 +      struct pair_pos_vs_id *a = (struct pair_pos_vs_id *)_a;
 +      struct pair_pos_vs_id *b = (struct pair_pos_vs_id *)_b;
 +
 +      return b->pack_int_id - a->pack_int_id;
 +}
 +
 +/*
 + * Limit calls to display_progress() for performance reasons.
 + * The interval here was arbitrarily chosen.
 + */
 +#define SPARSE_PROGRESS_INTERVAL (1 << 12)
 +#define midx_display_sparse_progress(progress, n) \
 +      do { \
 +              uint64_t _n = (n); \
 +              if ((_n & (SPARSE_PROGRESS_INTERVAL - 1)) == 0) \
 +                      display_progress(progress, _n); \
 +      } while (0)
 +
  int verify_midx_file(const char *object_dir)
  {
 +      struct pair_pos_vs_id *pairs = NULL;
        uint32_t i;
        struct progress *progress;
        struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
        if (!m)
                return 0;
  
 +      progress = start_progress(_("Looking for referenced packfiles"),
 +                                m->num_packs);
        for (i = 0; i < m->num_packs; i++) {
                if (prepare_midx_pack(m, i))
                        midx_report("failed to load pack in position %d", i);
 +
 +              display_progress(progress, i + 1);
        }
 +      stop_progress(&progress);
  
        for (i = 0; i < 255; i++) {
                uint32_t oid_fanout1 = ntohl(m->chunk_oid_fanout[i]);
                                    i, oid_fanout1, oid_fanout2, i + 1);
        }
  
 +      progress = start_sparse_progress(_("Verifying OID order in MIDX"),
 +                                       m->num_objects - 1);
        for (i = 0; i < m->num_objects - 1; i++) {
                struct object_id oid1, oid2;
  
                if (oidcmp(&oid1, &oid2) >= 0)
                        midx_report(_("oid lookup out of order: oid[%d] = %s >= %s = oid[%d]"),
                                    i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);
 +
 +              midx_display_sparse_progress(progress, i + 1);
        }
 +      stop_progress(&progress);
  
 -      progress = start_progress(_("Verifying object offsets"), m->num_objects);
 +      /*
 +       * Create an array mapping each object to its packfile id.  Sort it
 +       * to group the objects by packfile.  Use this permutation to visit
 +       * each of the objects and only require 1 packfile to be open at a
 +       * time.
 +       */
 +      ALLOC_ARRAY(pairs, m->num_objects);
 +      for (i = 0; i < m->num_objects; i++) {
 +              pairs[i].pos = i;
 +              pairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);
 +      }
 +
 +      progress = start_sparse_progress(_("Sorting objects by packfile"),
 +                                       m->num_objects);
 +      display_progress(progress, 0); /* TODO: Measure QSORT() progress */
 +      QSORT(pairs, m->num_objects, compare_pair_pos_vs_id);
 +      stop_progress(&progress);
 +
 +      progress = start_sparse_progress(_("Verifying object offsets"), m->num_objects);
        for (i = 0; i < m->num_objects; i++) {
                struct object_id oid;
                struct pack_entry e;
                off_t m_offset, p_offset;
  
 -              nth_midxed_object_oid(&oid, m, i);
 +              if (i > 0 && pairs[i-1].pack_int_id != pairs[i].pack_int_id &&
 +                  m->packs[pairs[i-1].pack_int_id])
 +              {
 +                      close_pack_fd(m->packs[pairs[i-1].pack_int_id]);
 +                      close_pack_index(m->packs[pairs[i-1].pack_int_id]);
 +              }
 +
 +              nth_midxed_object_oid(&oid, m, pairs[i].pos);
 +
                if (!fill_midx_entry(&oid, &e, m)) {
                        midx_report(_("failed to load pack entry for oid[%d] = %s"),
 -                                  i, oid_to_hex(&oid));
 +                                  pairs[i].pos, oid_to_hex(&oid));
                        continue;
                }
  
  
                if (m_offset != p_offset)
                        midx_report(_("incorrect object offset for oid[%d] = %s: %"PRIx64" != %"PRIx64),
 -                                  i, oid_to_hex(&oid), m_offset, p_offset);
 +                                  pairs[i].pos, oid_to_hex(&oid), m_offset, p_offset);
  
 -              display_progress(progress, i + 1);
 +              midx_display_sparse_progress(progress, i + 1);
        }
        stop_progress(&progress);
  
 +      free(pairs);
 +
        return verify_midx_error;
  }
diff --combined packfile.c
index d2bcb2f860c50056b7014b8a7dbf804baad79be8,7a2dd2fdbea4c750ad998fd6d6f9d95b3ddff060..b5ec0eeddd68cab7e1c5bd3bc58f4328fef049ab
@@@ -309,7 -309,7 +309,7 @@@ void close_pack_windows(struct packed_g
        }
  }
  
 -static int close_pack_fd(struct packed_git *p)
 +int close_pack_fd(struct packed_git *p)
  {
        if (p->pack_fd < 0)
                return 0;
@@@ -466,6 -466,16 +466,16 @@@ static unsigned int get_max_fd_limit(vo
  #endif
  }
  
+ const char *pack_basename(struct packed_git *p)
+ {
+       const char *ret = strrchr(p->pack_name, '/');
+       if (ret)
+               ret = ret + 1; /* skip past slash */
+       else
+               ret = p->pack_name; /* we only have a base */
+       return ret;
+ }
  /*
   * Do not call this directly as this leaks p->pack_fd on error return;
   * call open_packed_git() instead.
@@@ -482,7 -492,7 +492,7 @@@ static int open_packed_git_1(struct pac
  
        if (!p->index_data) {
                struct multi_pack_index *m;
-               const char *pack_name = strrchr(p->pack_name, '/');
+               const char *pack_name = pack_basename(p);
  
                for (m = the_repository->objects->multi_pack_index;
                     m; m = m->next) {
@@@ -2023,8 -2033,10 +2033,10 @@@ int for_each_object_in_pack(struct pack
        uint32_t i;
        int r = 0;
  
-       if (flags & FOR_EACH_OBJECT_PACK_ORDER)
-               load_pack_revindex(p);
+       if (flags & FOR_EACH_OBJECT_PACK_ORDER) {
+               if (load_pack_revindex(p))
+                       return -1;
+       }
  
        for (i = 0; i < p->num_objects; i++) {
                uint32_t pos;
diff --combined packfile.h
index b1c18504eb92cef76b7efca6790ba565a8f5e950,fe05fe0303f857374610d3b45c03494e46073d81..12baa6118a86216d76e342740c592802b70936df
@@@ -15,23 -15,29 +15,29 @@@ struct object_info
   *
   * Example: odb_pack_name(out, sha1, "idx") => ".git/objects/pack/pack-1234..idx"
   */
extern char *odb_pack_name(struct strbuf *buf, const unsigned char *sha1, const char *ext);
+ char *odb_pack_name(struct strbuf *buf, const unsigned char *sha1, const char *ext);
  
  /*
   * Return the name of the (local) packfile with the specified sha1 in
   * its name.  The return value is a pointer to memory that is
   * overwritten each time this function is called.
   */
extern char *sha1_pack_name(const unsigned char *sha1);
+ char *sha1_pack_name(const unsigned char *sha1);
  
  /*
   * Return the name of the (local) pack index file with the specified
   * sha1 in its name.  The return value is a pointer to memory that is
   * overwritten each time this function is called.
   */
extern char *sha1_pack_index_name(const unsigned char *sha1);
+ char *sha1_pack_index_name(const unsigned char *sha1);
  
- extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
+ /*
+  * Return the basename of the packfile, omitting any containing directory
+  * (e.g., "pack-1234abcd[...].pack").
+  */
+ const char *pack_basename(struct packed_git *p);
+ struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
  
  typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
                                      const char *file_pach, void *data);
@@@ -45,8 -51,8 +51,8 @@@ void for_each_file_in_pack_dir(const ch
  #define PACKDIR_FILE_GARBAGE 4
  extern void (*report_garbage)(unsigned seen_bits, const char *path);
  
extern void reprepare_packed_git(struct repository *r);
extern void install_packed_git(struct repository *r, struct packed_git *pack);
+ void reprepare_packed_git(struct repository *r);
+ void install_packed_git(struct repository *r, struct packed_git *pack);
  
  struct packed_git *get_packed_git(struct repository *r);
  struct list_head *get_packed_git_mru(struct repository *r);
@@@ -59,34 -65,32 +65,34 @@@ struct packed_git *get_all_packs(struc
   */
  unsigned long approximate_object_count(void);
  
extern struct packed_git *find_sha1_pack(const unsigned char *sha1,
-                                        struct packed_git *packs);
+ struct packed_git *find_sha1_pack(const unsigned char *sha1,
+                                 struct packed_git *packs);
  
extern void pack_report(void);
+ void pack_report(void);
  
  /*
   * mmap the index file for the specified packfile (if it is not
   * already mmapped).  Return 0 on success.
   */
extern int open_pack_index(struct packed_git *);
+ int open_pack_index(struct packed_git *);
  
  /*
   * munmap the index file for the specified packfile (if it is
   * currently mmapped).
   */
extern void close_pack_index(struct packed_git *);
+ void close_pack_index(struct packed_git *);
  
- extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);
 +int close_pack_fd(struct packed_git *p);
 +
+ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);
  
extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);
extern void close_pack_windows(struct packed_git *);
extern void close_pack(struct packed_git *);
extern void close_all_packs(struct raw_object_store *o);
extern void unuse_pack(struct pack_window **);
extern void clear_delta_base_cache(void);
extern struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
+ unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);
+ void close_pack_windows(struct packed_git *);
+ void close_pack(struct packed_git *);
+ void close_all_packs(struct raw_object_store *o);
+ void unuse_pack(struct pack_window **);
+ void clear_delta_base_cache(void);
+ struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
  
  /*
   * Make sure that a pointer access into an mmap'd index file is within bounds,
   * (like the 64-bit extended offset table), as we compare the size to the
   * fixed-length parts when we open the file.
   */
extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
+ void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
  
  /*
   * Perform binary search on a pack-index for a given oid. Packfile is expected to
@@@ -112,59 -116,59 +118,59 @@@ int bsearch_pack(const struct object_i
   * at the SHA-1 within the mmapped index.  Return NULL if there is an
   * error.
   */
extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
+ const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
  /*
   * Like nth_packed_object_sha1, but write the data into the object specified by
   * the the first argument.  Returns the first argument on success, and NULL on
   * error.
   */
extern const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
+ const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
  
  /*
   * Return the offset of the nth object within the specified packfile.
   * The index must already be opened.
   */
extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t n);
+ off_t nth_packed_object_offset(const struct packed_git *, uint32_t n);
  
  /*
   * If the object named sha1 is present in the specified packfile,
   * return its offset within the packfile; otherwise, return 0.
   */
extern off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *);
+ off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *);
  
extern int is_pack_valid(struct packed_git *);
extern void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
+ int is_pack_valid(struct packed_git *);
+ void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
+ unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
+ unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
+ int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
  
extern void release_pack_memory(size_t);
+ void release_pack_memory(size_t);
  
  /* global flag to enable extra checks when accessing packed objects */
  extern int do_check_packed_object_crc;
  
extern int packed_object_info(struct repository *r,
-                             struct packed_git *pack,
-                             off_t offset, struct object_info *);
+ int packed_object_info(struct repository *r,
+                      struct packed_git *pack,
+                      off_t offset, struct object_info *);
  
extern void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1);
extern const struct packed_git *has_packed_and_bad(struct repository *r, const unsigned char *sha1);
+ void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1);
+ const struct packed_git *has_packed_and_bad(struct repository *r, const unsigned char *sha1);
  
  /*
   * Iff a pack file in the given repository contains the object named by sha1,
   * return true and store its location to e.
   */
extern int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e);
+ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e);
  
extern int has_object_pack(const struct object_id *oid);
+ int has_object_pack(const struct object_id *oid);
  
extern int has_pack_index(const unsigned char *sha1);
+ int has_pack_index(const unsigned char *sha1);
  
  /*
   * Return 1 if an object in a promisor packfile is or refers to the given
   * object, 0 otherwise.
   */
extern int is_promisor_object(const struct object_id *oid);
+ int is_promisor_object(const struct object_id *oid);
  
  /*
   * Expose a function for fuzz testing.
   * have a convenient entry-point for fuzz testing. For real uses, you should
   * probably use open_pack_index() or parse_pack_index() instead.
   */
extern int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
-                   size_t idx_size, struct packed_git *p);
+ int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
+            size_t idx_size, struct packed_git *p);
  
  #endif
diff --combined t/t5570-git-daemon.sh
index 00fc612cacba4781975d066e01873e90e06050a8,19e271bda6a479a95dafc5ea9cdca0b2d3e0cf25..34487bbb8ce3c4d2548e2898f69fac0e3e7c4045
@@@ -90,6 -90,7 +90,7 @@@ test_expect_success 'fetch notices corr
  test_expect_success 'fetch notices corrupt idx' '
        cp -R "$GIT_DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$GIT_DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
        (cd "$GIT_DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
+        rm -f objects/pack/multi-pack-index &&
         p=$(ls objects/pack/pack-*.idx) &&
         chmod u+w $p &&
         printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
@@@ -198,4 -199,5 +199,4 @@@ test_expect_success FAKENC 'hostname in
        test_cmp expect actual
  '
  
 -stop_git_daemon
  test_done