{"thread":{"id":"35847","subject":"[PATCH] Make the global packed_git variable static to sha1_file.c.","startedAt":"2014-02-12T01:57:26Z","lastAt":"2014-02-12T19:28:55Z","messageCount":5,"participants":["Stefan Zager","Chris Packham","David Kastrup","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"234634","messageId":"20140212015727.1D63A403D3@wince.sfo.corp.google.com","threadId":"35847","inReplyTo":null,"subject":"[PATCH] Make the global packed_git variable static to sha1_file.c.","fromName":"Stefan Zager","fromEmail":"szager@google.com","sentAt":"2014-02-12T01:57:26Z","receivedAt":"2014-02-12T01:57:26Z","isPatch":true,"sender":{"key":"szager@google.com","avatar":null},"body":">From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001\nFrom: Stefan Zager <szager@chromium.org>\nDate: Mon, 10 Feb 2014 16:55:12 -0800\nSubject: [PATCH] Make the global packed_git variable static to sha1_file.c.\n\nThis is a first step in making the codebase thread-safe.  By and\nlarge, the operations which might benefit from threading are those\nthat work with pack files (e.g., checkout, blame), so the focus of\nthis patch is stop leaking the global list of pack files outside of\nsha1_file.c.\n\nThe next step will be to control access to the list of pack files\nwith a mutex.  However, that alone is not enough to make pack file\naccess thread safe.  Even in a read-only operation, the window list\nassociated with each pack file will need to be controlled.\nAdditionally, the global counters in sha1_file.c will need to be\ncontrolled.\n\nThis patch is a pure refactor with no functional changes, so it\nshouldn't require any additional tests.  Adding the actual locks\nwill be a functional change, and will require additional tests.\n\nSigned-off-by: Stefan Zager <szager@chromium.org>\n---\n builtin/count-objects.c  |  44 ++++++-----\n builtin/fsck.c           |  46 +++++++-----\n builtin/gc.c             |  26 +++----\n builtin/pack-objects.c   | 188 ++++++++++++++++++++++++++++-------------------\n builtin/pack-redundant.c |  37 +++++++---\n cache.h                  |  16 +++-\n fast-import.c            |   4 +-\n http-backend.c           |  28 ++++---\n http-push.c              |   4 +-\n http-walker.c            |   2 +-\n pack-revindex.c          |  20 ++---\n server-info.c            |  35 +++++----\n sha1_file.c              |  35 ++++++++-\n sha1_name.c              |  18 ++++-\n 14 files changed, 315 insertions(+), 188 deletions(-)\n\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex a7f70cb..5a8e487 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -83,14 +83,32 @@ static char const * const count_objects_usage[] = {\n \tNULL\n };\n \n+struct pack_data {\n+    unsigned long packed;\n+    off_t size_pack;\n+    unsigned long num_pack;\n+};\n+\n+int pack_data_fn(struct packed_git *p, void *data)\n+{\n+    struct pack_data *pd = (struct pack_data *) data;\n+    if (p->pack_local && !open_pack_index(p)) {\n+\t\tpd->packed += p->num_objects;\n+\t\tpd->size_pack += p->pack_size + p->index_size;\n+\t\tpd->num_pack++;\n+    }\n+    return 1;\n+}\n+\n int cmd_count_objects(int argc, const char **argv, const char *prefix)\n {\n \tint i, verbose = 0, human_readable = 0;\n \tconst char *objdir = get_object_directory();\n \tint len = strlen(objdir);\n \tchar *path = xmalloc(len + 50);\n-\tunsigned long loose = 0, packed = 0, packed_loose = 0;\n+\tunsigned long loose = 0, packed_loose = 0;\n \toff_t loose_size = 0;\n+    struct pack_data pd = {0,0,0};\n \tstruct option opts[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n \t\tOPT_BOOL('H', \"human-readable\", &human_readable,\n@@ -118,41 +136,29 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tclosedir(d);\n \t}\n \tif (verbose) {\n-\t\tstruct packed_git *p;\n-\t\tunsigned long num_pack = 0;\n-\t\toff_t size_pack = 0;\n \t\tstruct strbuf loose_buf = STRBUF_INIT;\n \t\tstruct strbuf pack_buf = STRBUF_INIT;\n \t\tstruct strbuf garbage_buf = STRBUF_INIT;\n-\t\tif (!packed_git)\n-\t\t\tprepare_packed_git();\n-\t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\tif (!p->pack_local)\n-\t\t\t\tcontinue;\n-\t\t\tif (open_pack_index(p))\n-\t\t\t\tcontinue;\n-\t\t\tpacked += p->num_objects;\n-\t\t\tsize_pack += p->pack_size + p->index_size;\n-\t\t\tnum_pack++;\n-\t\t}\n+\t\tprepare_packed_git();\n+\t\tforeach_packed_git(pack_data_fn, NULL, &pd);\n \n \t\tif (human_readable) {\n \t\t\tstrbuf_humanise_bytes(&loose_buf, loose_size);\n-\t\t\tstrbuf_humanise_bytes(&pack_buf, size_pack);\n+\t\t\tstrbuf_humanise_bytes(&pack_buf, pd.size_pack);\n \t\t\tstrbuf_humanise_bytes(&garbage_buf, size_garbage);\n \t\t} else {\n \t\t\tstrbuf_addf(&loose_buf, \"%lu\",\n \t\t\t\t    (unsigned long)(loose_size / 1024));\n \t\t\tstrbuf_addf(&pack_buf, \"%lu\",\n-\t\t\t\t    (unsigned long)(size_pack / 1024));\n+\t\t\t\t    (unsigned long)(pd.size_pack / 1024));\n \t\t\tstrbuf_addf(&garbage_buf, \"%lu\",\n \t\t\t\t    (unsigned long)(size_garbage / 1024));\n \t\t}\n \n \t\tprintf(\"count: %lu\\n\", loose);\n \t\tprintf(\"size: %s\\n\", loose_buf.buf);\n-\t\tprintf(\"in-pack: %lu\\n\", packed);\n-\t\tprintf(\"packs: %lu\\n\", num_pack);\n+\t\tprintf(\"in-pack: %lu\\n\", pd.packed);\n+\t\tprintf(\"packs: %lu\\n\", pd.num_pack);\n \t\tprintf(\"size-pack: %s\\n\", pack_buf.buf);\n \t\tprintf(\"prune-packable: %lu\\n\", packed_loose);\n \t\tprintf(\"garbage: %lu\\n\", garbage);\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 1affdd5..f9fe187 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -624,6 +624,29 @@ static struct option fsck_opts[] = {\n \tOPT_END(),\n };\n \n+struct verify_packs_data {\n+    struct progress *progress;\n+    uint32_t total;\n+    uint32_t count;\n+};\n+\n+static int count_packs_fn(struct packed_git *p, void *data)\n+{\n+    struct verify_packs_data *vpd = (struct verify_packs_data *) data;\n+    if (!open_pack_index(p))\n+\t\tvpd->total++;\n+    return 0;\n+}\n+\n+static int verify_packs_fn(struct packed_git *p, void *data)\n+{\n+    struct verify_packs_data *vpd = (struct verify_packs_data *) data;\n+    if (verify_pack(p, fsck_obj_buffer, vpd->progress, vpd->count))\n+\t\terrors_found |= ERROR_PACK;\n+    vpd->count += p->num_objects;\n+    return 0;\n+}\n+\n int cmd_fsck(int argc, const char **argv, const char *prefix)\n {\n \tint i, heads;\n@@ -657,29 +680,16 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (check_full) {\n-\t\tstruct packed_git *p;\n-\t\tuint32_t total = 0, count = 0;\n-\t\tstruct progress *progress = NULL;\n+\t\tstruct verify_packs_data vpd = {0,0,0};\n \n \t\tprepare_packed_git();\n \n \t\tif (show_progress) {\n-\t\t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\t\tif (open_pack_index(p))\n-\t\t\t\t\tcontinue;\n-\t\t\t\ttotal += p->num_objects;\n-\t\t\t}\n-\n-\t\t\tprogress = start_progress(\"Checking objects\", total);\n-\t\t}\n-\t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\t/* verify gives error messages itself */\n-\t\t\tif (verify_pack(p, fsck_obj_buffer,\n-\t\t\t\t\tprogress, count))\n-\t\t\t\terrors_found |= ERROR_PACK;\n-\t\t\tcount += p->num_objects;\n+\t\t\tforeach_packed_git(count_packs_fn, NULL, &vpd);\n+\t\t\tvpd.progress = start_progress(\"Checking objects\", vpd.total);\n \t\t}\n-\t\tstop_progress(&progress);\n+\t\tforeach_packed_git(verify_packs_fn, NULL, &vpd);\n+\t\tstop_progress(&vpd.progress);\n \t}\n \n \theads = 0;\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c19545d..9bd9742 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -125,26 +125,26 @@ static int too_many_loose_objects(void)\n \treturn needed;\n }\n \n+static int count_packs_fn(struct packed_git *p, void *data)\n+{\n+    /*\n+     * Perhaps check the size of the pack and count only\n+     * very small ones here?\n+     */\n+    if (p->pack_local && !p->pack_keep)\n+\t\t(*((int *) data))++;\n+    return 0;\n+}\n+\n static int too_many_packs(void)\n {\n-\tstruct packed_git *p;\n-\tint cnt;\n+\tint cnt = 0;\n \n \tif (gc_auto_pack_limit <= 0)\n \t\treturn 0;\n \n \tprepare_packed_git();\n-\tfor (cnt = 0, p = packed_git; p; p = p->next) {\n-\t\tif (!p->pack_local)\n-\t\t\tcontinue;\n-\t\tif (p->pack_keep)\n-\t\t\tcontinue;\n-\t\t/*\n-\t\t * Perhaps check the size of the pack and count only\n-\t\t * very small ones here?\n-\t\t */\n-\t\tcnt++;\n-\t}\n+    foreach_packed_git(count_packs_fn, NULL, &cnt);\n \treturn gc_auto_pack_limit <= cnt;\n }\n \ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 541667f..38248cf 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -900,14 +900,45 @@ static int no_try_delta(const char *path)\n \treturn 0;\n }\n \n+struct find_pack_data {\n+    const unsigned char *sha1;\n+    off_t offset;\n+    struct packed_git *found_pack;\n+    int exclude;\n+    int found_non_local_pack;\n+    int found_pack_keep;\n+};\n+\n+static int find_pack_fn(struct packed_git *p, void *data)\n+{\n+    struct find_pack_data *fpd = (struct find_pack_data *) data;\n+    off_t offset = find_pack_entry_one(fpd->sha1, p);\n+\tif (offset) {\n+\t\tif (!fpd->found_pack) {\n+\t\t\tif (!is_pack_valid(p)) {\n+\t\t\t\twarning(\"packfile %s cannot be accessed\", p->pack_name);\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t\tfpd->offset = offset;\n+\t\t\tfpd->found_pack = p;\n+\t\t}\n+\t\tif (fpd->exclude)\n+\t\t\treturn 1;\n+\t\tif (!p->pack_local)\n+\t\t\tfpd->found_non_local_pack = 1;\n+\t\telse if (p->pack_keep)\n+\t\t\tfpd->found_pack_keep = 1;\n+\t}\n+\treturn 0;\n+}\n+\n static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t    const char *name, int exclude)\n {\n \tstruct object_entry *entry;\n-\tstruct packed_git *p, *found_pack = NULL;\n-\toff_t found_offset = 0;\n \tint ix;\n \tuint32_t hash = name_hash(name);\n+    struct find_pack_data fpd;\n \n \tix = nr_objects ? locate_object_entry_hash(sha1) : -1;\n \tif (ix >= 0) {\n@@ -923,27 +954,18 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \tif (!exclude && local && has_loose_object_nonlocal(sha1))\n \t\treturn 0;\n \n-\tfor (p = packed_git; p; p = p->next) {\n-\t\toff_t offset = find_pack_entry_one(sha1, p);\n-\t\tif (offset) {\n-\t\t\tif (!found_pack) {\n-\t\t\t\tif (!is_pack_valid(p)) {\n-\t\t\t\t\twarning(\"packfile %s cannot be accessed\", p->pack_name);\n-\t\t\t\t\tcontinue;\n-\t\t\t\t}\n-\t\t\t\tfound_offset = offset;\n-\t\t\t\tfound_pack = p;\n-\t\t\t}\n-\t\t\tif (exclude)\n-\t\t\t\tbreak;\n-\t\t\tif (incremental)\n-\t\t\t\treturn 0;\n-\t\t\tif (local && !p->pack_local)\n-\t\t\t\treturn 0;\n-\t\t\tif (ignore_packed_keep && p->pack_local && p->pack_keep)\n-\t\t\t\treturn 0;\n-\t\t}\n-\t}\n+    fpd.sha1 = sha1;\n+    fpd.offset = 0;\n+    fpd.found_pack = NULL;\n+    fpd.exclude = exclude;\n+    fpd.found_non_local_pack = fpd.found_pack_keep = 0;\n+\n+    foreach_packed_git(find_pack_fn, NULL, &fpd);\n+\tif (fpd.found_pack && !exclude &&\n+\t\t(incremental ||\n+\t\t (local && fpd.found_non_local_pack) ||\n+\t\t (ignore_packed_keep && fpd.found_pack_keep)))\n+\t\treturn 0;\n \n \tif (nr_objects >= nr_alloc) {\n \t\tnr_alloc = (nr_alloc  + 1024) * 3 / 2;\n@@ -960,9 +982,9 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\tentry->preferred_base = 1;\n \telse\n \t\tnr_result++;\n-\tif (found_pack) {\n-\t\tentry->in_pack = found_pack;\n-\t\tentry->in_pack_offset = found_offset;\n+\tif (fpd.found_pack) {\n+\t\tentry->in_pack = fpd.found_pack;\n+\t\tentry->in_pack_offset = fpd.offset;\n \t}\n \n \tif (object_ix_hashsz * 3 <= nr_objects * 4)\n@@ -2257,35 +2279,40 @@ static int ofscmp(const void *a_, const void *b_)\n \t\treturn hashcmp(a->object->sha1, b->object->sha1);\n }\n \n-static void add_objects_in_unpacked_packs(struct rev_info *revs)\n+static int add_objects_fn(struct packed_git *p, void *data)\n {\n-\tstruct packed_git *p;\n-\tstruct in_pack in_pack;\n+    struct in_pack *ip = (struct in_pack *) data;\n \tuint32_t i;\n+    const unsigned char *sha1;\n+    struct object *o;\n \n-\tmemset(&in_pack, 0, sizeof(in_pack));\n-\n-\tfor (p = packed_git; p; p = p->next) {\n-\t\tconst unsigned char *sha1;\n-\t\tstruct object *o;\n-\n-\t\tif (!p->pack_local || p->pack_keep)\n-\t\t\tcontinue;\n+\tif (p->pack_local && !p->pack_keep) {\n \t\tif (open_pack_index(p))\n \t\t\tdie(\"cannot open pack index\");\n \n-\t\tALLOC_GROW(in_pack.array,\n-\t\t\t   in_pack.nr + p->num_objects,\n-\t\t\t   in_pack.alloc);\n+\t\tALLOC_GROW(ip->array,\n+\t\t\t\t   ip->nr + p->num_objects,\n+\t\t\t\t   ip->alloc);\n \n \t\tfor (i = 0; i < p->num_objects; i++) {\n \t\t\tsha1 = nth_packed_object_sha1(p, i);\n \t\t\to = lookup_unknown_object(sha1);\n \t\t\tif (!(o->flags & OBJECT_ADDED))\n-\t\t\t\tmark_in_pack_object(o, p, &in_pack);\n+\t\t\t\tmark_in_pack_object(o, p, ip);\n \t\t\to->flags |= OBJECT_ADDED;\n \t\t}\n \t}\n+\treturn 0;\n+}\n+\n+static void add_objects_in_unpacked_packs(struct rev_info *revs)\n+{\n+\tstruct in_pack in_pack;\n+    uint32_t i;\n+\n+\tmemset(&in_pack, 0, sizeof(in_pack));\n+\n+    foreach_packed_git(add_objects_fn, NULL, &in_pack);\n \n \tif (in_pack.nr) {\n \t\tqsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),\n@@ -2298,54 +2325,63 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)\n \tfree(in_pack.array);\n }\n \n+struct nonlocal_or_keep_data {\n+    struct packed_git **last_found;\n+    const unsigned char *sha1;\n+    int found;\n+};\n+\n+static int find_nonlocal_or_keep_fn(struct packed_git *p, void *data)\n+{\n+    struct nonlocal_or_keep_data *d = (struct nonlocal_or_keep_data *) data;\n+\tif ((!p->pack_local || p->pack_keep) &&\n+\t\tfind_pack_entry_one(d->sha1, p)) {\n+\t\t*d->last_found = p;\n+\t\td->found = 1;\n+\t\treturn 1;\n+\t}\n+    return 0;\n+}\n+\n static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)\n {\n \tstatic struct packed_git *last_found = (void *)1;\n-\tstruct packed_git *p;\n+    struct nonlocal_or_keep_data d;\n+    struct packed_git *hint;\n \n-\tp = (last_found != (void *)1) ? last_found : packed_git;\n+    d.last_found = &last_found;\n+    d.sha1 = sha1;\n+    d.found = 0;\n \n-\twhile (p) {\n-\t\tif ((!p->pack_local || p->pack_keep) &&\n-\t\t\tfind_pack_entry_one(sha1, p)) {\n-\t\t\tlast_found = p;\n-\t\t\treturn 1;\n-\t\t}\n-\t\tif (p == last_found)\n-\t\t\tp = packed_git;\n-\t\telse\n-\t\t\tp = p->next;\n-\t\tif (p == last_found)\n-\t\t\tp = p->next;\n-\t}\n-\treturn 0;\n+\thint = (last_found != (void *)1) ? last_found : NULL;\n+\n+    foreach_packed_git(find_nonlocal_or_keep_fn, hint, &d);\n+    return d.found;\n }\n \n-static void loosen_unused_packed_objects(struct rev_info *revs)\n+static int loosen_unused_objects_fn(struct packed_git *p, void *data)\n {\n-\tstruct packed_git *p;\n+\tconst unsigned char *sha1 = (const unsigned char *) data;\n \tuint32_t i;\n-\tconst unsigned char *sha1;\n \n-\tfor (p = packed_git; p; p = p->next) {\n-\t\tif (!p->pack_local || p->pack_keep)\n-\t\t\tcontinue;\n+\tif (!p->pack_local || p->pack_keep)\n+\t\treturn 0;\n \n-\t\tif (unpack_unreachable_expiration &&\n-\t\t    p->mtime < unpack_unreachable_expiration)\n-\t\t\tcontinue;\n+\tif (unpack_unreachable_expiration &&\n+\t\tp->mtime < unpack_unreachable_expiration)\n+\t\treturn 0;\n \n-\t\tif (open_pack_index(p))\n-\t\t\tdie(\"cannot open pack index\");\n+\tif (open_pack_index(p))\n+\t\tdie(\"cannot open pack index\");\n \n-\t\tfor (i = 0; i < p->num_objects; i++) {\n-\t\t\tsha1 = nth_packed_object_sha1(p, i);\n-\t\t\tif (!locate_object_entry(sha1) &&\n-\t\t\t\t!has_sha1_pack_kept_or_nonlocal(sha1))\n-\t\t\t\tif (force_object_loose(sha1, p->mtime))\n-\t\t\t\t\tdie(\"unable to force loose object\");\n-\t\t}\n+\tfor (i = 0; i < p->num_objects; i++) {\n+\t\tsha1 = nth_packed_object_sha1(p, i);\n+\t\tif (!locate_object_entry(sha1) &&\n+\t\t\t!has_sha1_pack_kept_or_nonlocal(sha1))\n+\t\t\tif (force_object_loose(sha1, p->mtime))\n+\t\t\t\tdie(\"unable to force loose object\");\n \t}\n+\treturn 0;\n }\n \n static void get_object_list(int ac, const char **av)\n@@ -2383,7 +2419,7 @@ static void get_object_list(int ac, const char **av)\n \tif (keep_unreachable)\n \t\tadd_objects_in_unpacked_packs(&revs);\n \tif (unpack_unreachable)\n-\t\tloosen_unused_packed_objects(&revs);\n+\t\tforeach_packed_git(loosen_unused_objects_fn, NULL, NULL);\n }\n \n static int option_parse_index_version(const struct option *opt,\ndiff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c\nindex 649c3aa..7dbaeba 100644\n--- a/builtin/pack-redundant.c\n+++ b/builtin/pack-redundant.c\n@@ -567,29 +567,42 @@ static struct pack_list * add_pack(struct packed_git *p)\n \t\treturn pack_list_insert(&altodb_packs, &l);\n }\n \n+struct add_pack_data {\n+    const char *filename;\n+    int found;\n+    struct pack_list *added_pack;\n+};\n+\n+static int add_pack_fn(struct packed_git *p, void *data)\n+{\n+\tstruct add_pack_data *apd = (struct add_pack_data *) data;\n+\tif (apd->filename && strstr(p->pack_name, apd->filename)) {\n+\t\tapd->found = 1;\n+\t\tapd->added_pack = add_pack(p);\n+\t\treturn 1;\n+\t} else if (!apd->filename) {\n+\t\tadd_pack(p);\n+\t}\n+\treturn 0;\n+}\n+\n static struct pack_list * add_pack_file(const char *filename)\n {\n-\tstruct packed_git *p = packed_git;\n+    struct add_pack_data apd = {filename,0,NULL};\n \n \tif (strlen(filename) < 40)\n \t\tdie(\"Bad pack filename: %s\", filename);\n \n-\twhile (p) {\n-\t\tif (strstr(p->pack_name, filename))\n-\t\t\treturn add_pack(p);\n-\t\tp = p->next;\n-\t}\n+    foreach_packed_git(add_pack_fn, NULL, &apd);\n+\tif (apd.found)\n+\t\treturn apd.added_pack;\n \tdie(\"Filename %s not found in packed_git\", filename);\n }\n \n static void load_all(void)\n {\n-\tstruct packed_git *p = packed_git;\n-\n-\twhile (p) {\n-\t\tadd_pack(p);\n-\t\tp = p->next;\n-\t}\n+    struct add_pack_data apd = {NULL,0,NULL};\n+    foreach_packed_git(add_pack_fn, NULL, &apd);\n }\n \n int cmd_pack_redundant(int argc, const char **argv, const char *prefix)\ndiff --git a/cache.h b/cache.h\nindex dc040fb..542a9d9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1044,7 +1044,7 @@ struct pack_window {\n \tunsigned int inuse_cnt;\n };\n \n-extern struct packed_git {\n+struct packed_git {\n \tstruct packed_git *next;\n \tstruct pack_window *windows;\n \toff_t pack_size;\n@@ -1062,7 +1062,7 @@ extern struct packed_git {\n \tunsigned char sha1[20];\n \t/* something like \".git/objects/pack/xxxxx.pack\" */\n \tchar pack_name[FLEX_ARRAY]; /* more */\n-} *packed_git;\n+};\n \n struct pack_entry {\n \toff_t offset;\n@@ -1070,6 +1070,18 @@ struct pack_entry {\n \tstruct packed_git *p;\n };\n \n+/* packed_git_foreach_fn implementations should return zero\n+ * to continue the traversal, non-zero to halt.\n+ */\n+typedef int (*packed_git_foreach_fn)(struct packed_git *, void *);\n+\n+/* The 'hint' argument is for the commonly-used 'last found pack' optimization.\n+ * It can be NULL.\n+ */\n+extern void foreach_packed_git(packed_git_foreach_fn fn, struct packed_git *hint, void *data);\n+\n+extern size_t packed_git_count();\n+extern size_t packed_git_local_count();\n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \n /* A hook for count-objects to report invalid files in pack directory */\ndiff --git a/fast-import.c b/fast-import.c\nindex 4fd18a3..11d1f22 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1037,7 +1037,7 @@ static int store_object(\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n-\t} else if (find_sha1_pack(sha1, packed_git)) {\n+\t} else if (find_sha1_pack(sha1, NULL)) {\n \t\te->type = type;\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\n@@ -1238,7 +1238,7 @@ static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n \t\ttruncate_pack(&checkpoint);\n \n-\t} else if (find_sha1_pack(sha1, packed_git)) {\n+\t} else if (find_sha1_pack(sha1, NULL)) {\n \t\te->type = OBJ_BLOB;\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\ndiff --git a/http-backend.c b/http-backend.c\nindex d2c0a62..f19efcb 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -438,25 +438,29 @@ static void get_head(char *arg)\n \tstrbuf_release(&buf);\n }\n \n+struct pack_info_data {\n+    struct strbuf *buf;\n+    size_t objdirlen;\n+};\n+\n+static int append_pack_info_fn(struct packed_git *p, void *data)\n+{\n+\tstruct pack_info_data *d = (struct pack_info_data *) data;\n+\tif (p->pack_local)\n+\t\tstrbuf_addf(d->buf, \"P %s\\n\", p->pack_name + d->objdirlen + 6);\n+\treturn 0;\n+}\n+\n static void get_info_packs(char *arg)\n {\n \tsize_t objdirlen = strlen(get_object_directory());\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct packed_git *p;\n-\tsize_t cnt = 0;\n+    struct pack_info_data d = {&buf, objdirlen};\n \n \tselect_getanyfile();\n \tprepare_packed_git();\n-\tfor (p = packed_git; p; p = p->next) {\n-\t\tif (p->pack_local)\n-\t\t\tcnt++;\n-\t}\n-\n-\tstrbuf_grow(&buf, cnt * 53 + 2);\n-\tfor (p = packed_git; p; p = p->next) {\n-\t\tif (p->pack_local)\n-\t\t\tstrbuf_addf(&buf, \"P %s\\n\", p->pack_name + objdirlen + 6);\n-\t}\n+\tstrbuf_grow(&buf, packed_git_local_count() * 53 + 2);\n+    foreach_packed_git(append_pack_info_fn, NULL, &d);\n \tstrbuf_addch(&buf, '\\n');\n \n \thdr_nocache();\ndiff --git a/http-push.c b/http-push.c\nindex d4b40c9..f2c4256 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -304,7 +304,7 @@ static void start_fetch_packed(struct transfer_request *request)\n \tstruct transfer_request *check_request = request_queue_head;\n \tstruct http_pack_request *preq;\n \n-\ttarget = find_sha1_pack(request->obj->sha1, repo->packs);\n+\ttarget = repo->packs ? find_sha1_pack(request->obj->sha1, repo->packs) : NULL;\n \tif (!target) {\n \t\tfprintf(stderr, \"Unable to fetch %s, will not be able to update server info refs\\n\", sha1_to_hex(request->obj->sha1));\n \t\trepo->can_update_info_refs = 0;\n@@ -677,7 +677,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock)\n \t\tget_remote_object_list(obj->sha1[0]);\n \tif (obj->flags & (REMOTE | PUSHING))\n \t\treturn 0;\n-\ttarget = find_sha1_pack(obj->sha1, repo->packs);\n+\ttarget = repo->packs ? find_sha1_pack(obj->sha1, repo->packs) : NULL;\n \tif (target) {\n \t\tobj->flags |= REMOTE;\n \t\treturn 0;\ndiff --git a/http-walker.c b/http-walker.c\nindex 1516c5e..d9d758a 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -405,7 +405,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne\n \n \tif (fetch_indices(walker, repo))\n \t\treturn -1;\n-\ttarget = find_sha1_pack(sha1, repo->packs);\n+\ttarget = repo->packs ? find_sha1_pack(sha1, repo->packs) : NULL;\n \tif (!target)\n \t\treturn -1;\n \ndiff --git a/pack-revindex.c b/pack-revindex.c\nindex b4d2b35..902223b 100644\n--- a/pack-revindex.c\n+++ b/pack-revindex.c\n@@ -40,23 +40,25 @@ static int pack_revindex_ix(struct packed_git *p)\n \treturn -1 - i;\n }\n \n+static int init_pack_revindex_fn(struct packed_git *p, void *data)\n+{\n+\t/* revindex elements are lazily initialized */\n+    int num = pack_revindex_ix(p);\n+    num = - 1 - num;\n+    pack_revindex[num].p = p;\n+    return 0;\n+}\n+\n static void init_pack_revindex(void)\n {\n \tint num;\n-\tstruct packed_git *p;\n \n-\tfor (num = 0, p = packed_git; p; p = p->next)\n-\t\tnum++;\n+    num = packed_git_count();\n \tif (!num)\n \t\treturn;\n \tpack_revindex_hashsz = num * 11;\n \tpack_revindex = xcalloc(sizeof(*pack_revindex), pack_revindex_hashsz);\n-\tfor (p = packed_git; p; p = p->next) {\n-\t\tnum = pack_revindex_ix(p);\n-\t\tnum = - 1 - num;\n-\t\tpack_revindex[num].p = p;\n-\t}\n-\t/* revindex elements are lazily initialized */\n+    foreach_packed_git(init_pack_revindex_fn, NULL, NULL);\n }\n \n /*\ndiff --git a/server-info.c b/server-info.c\nindex 9ec744e..5eb3773 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -151,9 +151,20 @@ static int compare_info(const void *a_, const void *b_)\n \t\treturn 1;\n }\n \n+static int init_pack_info_fn(struct packed_git *p, void *data)\n+{\n+    int *idx = (int *) data;\n+    if (p->pack_local) {\n+\t\tinfo[*idx] = xcalloc(1, sizeof(struct pack_info));\n+\t\tinfo[*idx]->p = p;\n+\t\tinfo[*idx]->old_num = -1;\n+\t\t(*idx)++;\n+    }\n+    return 0;\n+}\n+\n static void init_pack_info(const char *infofile, int force)\n {\n-\tstruct packed_git *p;\n \tint stale;\n \tint i = 0;\n \n@@ -161,24 +172,12 @@ static void init_pack_info(const char *infofile, int force)\n \tobjdirlen = strlen(objdir);\n \n \tprepare_packed_git();\n-\tfor (p = packed_git; p; p = p->next) {\n-\t\t/* we ignore things on alternate path since they are\n-\t\t * not available to the pullers in general.\n-\t\t */\n-\t\tif (!p->pack_local)\n-\t\t\tcontinue;\n-\t\ti++;\n-\t}\n-\tnum_pack = i;\n+    /* we ignore things on alternate path since they are\n+     * not available to the pullers in general.\n+     */\n+    num_pack = packed_git_local_count();\n \tinfo = xcalloc(num_pack, sizeof(struct pack_info *));\n-\tfor (i = 0, p = packed_git; p; p = p->next) {\n-\t\tif (!p->pack_local)\n-\t\t\tcontinue;\n-\t\tinfo[i] = xcalloc(1, sizeof(struct pack_info));\n-\t\tinfo[i]->p = p;\n-\t\tinfo[i]->old_num = -1;\n-\t\ti++;\n-\t}\n+    foreach_packed_git(init_pack_info_fn, NULL, &i);\n \n \tif (infofile && !force)\n \t\tstale = read_pack_info_file(infofile);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 6e8c05d..aeeb7e6 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -60,6 +60,7 @@ static struct cached_object empty_tree = {\n \t0\n };\n \n+static struct packed_git *packed_git;\n static struct packed_git *last_found_pack;\n \n static struct cached_object *find_cached_object(const unsigned char *sha1)\n@@ -468,7 +469,6 @@ static unsigned int pack_open_fds;\n static unsigned int pack_max_fds;\n static size_t peak_pack_mapped;\n static size_t pack_mapped;\n-struct packed_git *packed_git;\n \n void pack_report(void)\n {\n@@ -1091,6 +1091,37 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)\n \treturn p;\n }\n \n+void foreach_packed_git(packed_git_foreach_fn fn, struct packed_git *hint, void *data)\n+{\n+\tstruct packed_git *p;\n+\tif (hint && ((*fn)(hint, data)))\n+\t\treturn;\n+\tfor (p = packed_git; p; p = p->next)\n+\t\tif (p != hint && (*fn)(p, data))\n+\t\t\treturn;\n+}\n+\n+size_t packed_git_count()\n+{\n+\tsize_t res = 0;\n+\tstruct packed_git *p;\n+\n+\tfor (p = packed_git; p; p = p->next)\n+\t\t++res;\n+\treturn res;\n+}\n+\n+size_t packed_git_local_count()\n+{\n+\tsize_t res = 0;\n+\tstruct packed_git *p;\n+\n+\tfor (p = packed_git; p; p = p->next)\n+\t\tif (p->pack_local)\n+\t\t\t++res;\n+\treturn res;\n+}\n+\n struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path)\n {\n \tconst char *path = sha1_pack_name(sha1);\n@@ -2474,6 +2505,8 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1,\n {\n \tstruct packed_git *p;\n \n+\tif (!packs)\n+\t\tpacks = packed_git;\n \tfor (p = packs; p; p = p->next) {\n \t\tif (find_pack_entry_one(sha1, p))\n \t\t\treturn p;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 6fca869..bc6d456 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -176,14 +176,26 @@ static void unique_in_pack(int len,\n \t}\n }\n \n+struct disambiguate_data {\n+    int len;\n+    const unsigned char *bin_pfx;\n+    struct disambiguate_state *ds;\n+};\n+\n+static int find_short_packed_object_fn(struct packed_git *p, void *data)\n+{\n+    struct disambiguate_data *d = (struct disambiguate_data *) data;\n+    unique_in_pack(d->len, d->bin_pfx, p, d->ds);\n+    return 0;\n+}\n+\n static void find_short_packed_object(int len, const unsigned char *bin_pfx,\n \t\t\t\t     struct disambiguate_state *ds)\n {\n-\tstruct packed_git *p;\n+    struct disambiguate_data d = {len,bin_pfx,ds};\n \n \tprepare_packed_git();\n-\tfor (p = packed_git; p && !ds->ambiguous; p = p->next)\n-\t\tunique_in_pack(len, bin_pfx, p, ds);\n+    foreach_packed_git(find_short_packed_object_fn, NULL, &d);\n }\n \n #define SHORT_NAME_NOT_FOUND (-1)\n-- \n1.9.0.rc3.1.g9c2198e.dirty\n"},{"id":"234643","messageId":"52FB22F3.8070100@gmail.com","threadId":"35847","inReplyTo":"20140212015727.1D63A403D3@wince.sfo.corp.google.com","subject":"Re: [PATCH] Make the global packed_git variable static to sha1_file.c.","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-02-12T07:29:55Z","receivedAt":"2014-02-12T07:29:55Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi,\n\nOn 12/02/14 14:57, Stefan Zager wrote:\n> From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001\n> From: Stefan Zager <szager@chromium.org>\n> Date: Mon, 10 Feb 2014 16:55:12 -0800\n> Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.\n> \n> This is a first step in making the codebase thread-safe.  By and\n> large, the operations which might benefit from threading are those\n> that work with pack files (e.g., checkout, blame), so the focus of\n> this patch is stop leaking the global list of pack files outside of\n> sha1_file.c.\n> \n> The next step will be to control access to the list of pack files\n> with a mutex.  However, that alone is not enough to make pack file\n> access thread safe.  Even in a read-only operation, the window list\n> associated with each pack file will need to be controlled.\n> Additionally, the global counters in sha1_file.c will need to be\n> controlled.\n> \n> This patch is a pure refactor with no functional changes, so it\n> shouldn't require any additional tests.  Adding the actual locks\n> will be a functional change, and will require additional tests.\n> \n> Signed-off-by: Stefan Zager <szager@chromium.org>\n> ---\n>  builtin/count-objects.c  |  44 ++++++-----\n>  builtin/fsck.c           |  46 +++++++-----\n>  builtin/gc.c             |  26 +++----\n>  builtin/pack-objects.c   | 188 ++++++++++++++++++++++++++++-------------------\n>  builtin/pack-redundant.c |  37 +++++++---\n>  cache.h                  |  16 +++-\n>  fast-import.c            |   4 +-\n>  http-backend.c           |  28 ++++---\n>  http-push.c              |   4 +-\n>  http-walker.c            |   2 +-\n>  pack-revindex.c          |  20 ++---\n>  server-info.c            |  35 +++++----\n>  sha1_file.c              |  35 ++++++++-\n>  sha1_name.c              |  18 ++++-\n>  14 files changed, 315 insertions(+), 188 deletions(-)\n\nI'm not really qualified to comment on substance but there are some\nbasic style issues w.r.t. whitespace namely using 4 spaces for indent\nand mixing tabs/spaces. This might seem pedantic for the first round of\na patch but it does put off reviewers.\n\n>From Documentation/CodingGuidelines:\n\n - We use tabs to indent, and interpret tabs as taking up to\n   8 spaces.\n"},{"id":"234670","messageId":"CAHOQ7J-2BkQOr+_BF42ja4pWaUWkt8OC-YE0ETwHAYsmrubi=A@mail.gmail.com","threadId":"35847","inReplyTo":"52FB22F3.8070100@gmail.com","subject":"Re: [PATCH] Make the global packed_git variable static to sha1_file.c.","fromName":"Stefan Zager","fromEmail":"szager@google.com","sentAt":"2014-02-12T18:26:22Z","receivedAt":"2014-02-12T18:26:22Z","isPatch":true,"sender":{"key":"szager@google.com","avatar":null},"body":"On Tue, Feb 11, 2014 at 11:29 PM, Chris Packham <judge.packham@gmail.com> wrote:\n> Hi,\n>\n> On 12/02/14 14:57, Stefan Zager wrote:\n>> From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001\n>> From: Stefan Zager <szager@chromium.org>\n>> Date: Mon, 10 Feb 2014 16:55:12 -0800\n>> Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.\n>\n> I'm not really qualified to comment on substance but there are some\n> basic style issues w.r.t. whitespace namely using 4 spaces for indent\n> and mixing tabs/spaces. This might seem pedantic for the first round of\n> a patch but it does put off reviewers.\n>\n> From Documentation/CodingGuidelines:\n>\n>  - We use tabs to indent, and interpret tabs as taking up to\n>    8 spaces.\n\nMy bad, I will upload a fixed patch.  In my defense: I edited the code\nin emacs and then ran \"M-x tabify\" over the entire file.  But that had\nthe unfortunate side effect of adding a bunch of whitespace-only\nchanges to the diff, illuminating the fact that there is already mixed\nwhitespace in the existing code.  So I had to go back and selectively\ntabify my changes, and I clearly missed a bunch.\n\nIf anyone has a recommendation for a less labor-intensive way to do\nthis in emacs, I'd be very grateful.\n\nThanks,\n\nStefan\n"},{"id":"234677","messageId":"87txc488o7.fsf@fencepost.gnu.org","threadId":"35847","inReplyTo":"CAHOQ7J-2BkQOr+_BF42ja4pWaUWkt8OC-YE0ETwHAYsmrubi=A@mail.gmail.com","subject":"Re: [PATCH] Make the global packed_git variable static to sha1_file.c.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-12T18:53:12Z","receivedAt":"2014-02-12T18:53:12Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Stefan Zager <szager@google.com> writes:\n\n> On Tue, Feb 11, 2014 at 11:29 PM, Chris Packham <judge.packham@gmail.com> wrote:\n>> Hi,\n>>\n>> On 12/02/14 14:57, Stefan Zager wrote:\n>>> From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001\n>>> From: Stefan Zager <szager@chromium.org>\n>>> Date: Mon, 10 Feb 2014 16:55:12 -0800\n>>> Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.\n>>\n>> I'm not really qualified to comment on substance but there are some\n>> basic style issues w.r.t. whitespace namely using 4 spaces for indent\n>> and mixing tabs/spaces. This might seem pedantic for the first round of\n>> a patch but it does put off reviewers.\n>>\n>> From Documentation/CodingGuidelines:\n>>\n>>  - We use tabs to indent, and interpret tabs as taking up to\n>>    8 spaces.\n>\n> My bad, I will upload a fixed patch.  In my defense: I edited the code\n> in emacs and then ran \"M-x tabify\" over the entire file.  But that had\n> the unfortunate side effect of adding a bunch of whitespace-only\n> changes to the diff, illuminating the fact that there is already mixed\n> whitespace in the existing code.  So I had to go back and selectively\n> tabify my changes, and I clearly missed a bunch.\n>\n> If anyone has a recommendation for a less labor-intensive way to do\n> this in emacs, I'd be very grateful.\n\nC-c . RET linux RET before entering any changes.\n\n-- \nDavid Kastrup\n"},{"id":"234683","messageId":"xmqq1tz8nn9k.fsf@gitster.dls.corp.google.com","threadId":"35847","inReplyTo":"CAHOQ7J-2BkQOr+_BF42ja4pWaUWkt8OC-YE0ETwHAYsmrubi=A@mail.gmail.com","subject":"Re: [PATCH] Make the global packed_git variable static to sha1_file.c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-12T19:28:55Z","receivedAt":"2014-02-12T19:28:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Zager <szager@google.com> writes:\n\n> If anyone has a recommendation for a less labor-intensive way to do\n> this in emacs, I'd be very grateful.\n\nThis is not \"do this in emacs\", but here is a possible approach.\n\nYou can ask \"git diff\" about what you changed, and actually apply\nthe change while fixing whitespace errors.  I.e.\n\n\tgit diff sha1_file.c | git apply --cached --whitespace=fix\n\tgit diff\n\tgit checkout sha1_file.c\n\nThe first step will add a cleaned-up version to your index.\n\nThe second \"diff\" (optional) is to see what whitespace errors are\nintroduced when going from that cleaned-up version to what you have\nin the working tree.\n\nWith the last step you would update the working tree version to the\ncleaned-up version from the index.\n\n\t[alias]\n\twsadd = \"!sh -c 'git diff -- \\\"$@\\\" | git apply --cached --whitespace=fix;\\\n\t\tgit co -- ${1-.} \\\"$@\\\"' -\"\n"}]}