From: Junio C Hamano Date: Thu, 25 Apr 2019 07:41:13 +0000 (+0900) Subject: Merge branch 'jk/server-info-rabbit-hole' X-Git-Tag: v2.22.0-rc0~70 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/776f3e1fb73a6440804aa3dde4ffd3b6bdf60a19?ds=inline;hp=-c Merge branch 'jk/server-info-rabbit-hole' 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 --- 776f3e1fb73a6440804aa3dde4ffd3b6bdf60a19 diff --combined http.c index 89fcd36a80,2ef47bc779..b5833550ca --- a/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 @@@ -1555,24 -1554,19 +1555,24 @@@ * 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"); @@@ -2169,24 -2163,17 +2169,17 @@@ 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 f1137eb6ee,0ceca1938f..d5d2e9522f --- a/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; @@@ -165,9 -164,6 +165,9 @@@ 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; @@@ -321,7 -349,7 +353,7 @@@ 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); @@@ -999,15 -1000,10 +1031,15 @@@ 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]); @@@ -1018,8 -1014,6 +1050,8 @@@ 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; @@@ -1029,47 -1023,18 +1061,47 @@@ 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; } @@@ -1084,13 -1049,11 +1116,13 @@@ 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 d2bcb2f860,7a2dd2fdbe..b5ec0eeddd --- a/packfile.c +++ b/packfile.c @@@ -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 b1c18504eb,fe05fe0303..12baa6118a --- a/packfile.h +++ b/packfile.h @@@ -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 *); +int close_pack_fd(struct packed_git *p); + - extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value); + 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, @@@ -96,7 -100,7 +102,7 @@@ * (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. @@@ -176,7 -180,7 +182,7 @@@ * 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 00fc612cac,19e271bda6..34487bbb8c --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@@ -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