{"thread":{"id":"9939","subject":"[PATCH 3/5] Always obtain fetch-pack arguments from struct fetch_pack_args","startedAt":"2007-09-19T04:49:35Z","lastAt":"2007-09-19T04:49:35Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"53549","messageId":"20070919044935.GC17107@spearce.org","threadId":"9939","inReplyTo":null,"subject":"[PATCH 3/5] Always obtain fetch-pack arguments from struct fetch_pack_args","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-19T04:49:35Z","receivedAt":"2007-09-19T04:49:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Copying the arguments from a fetch_pack_args into static globals\nwithin the builtin-fetch-pack module is error-prone and may lead\nrise to cases where arguments supplied via the struct from the\nnew fetch_pack() API may not be honored by the implementation.\n\nHere we reorganize all of the static globals into a single static\nstruct fetch_pack_args instance and use memcpy() to move the data\nfrom the caller supplied structure into the globals before we\nexecute our pack fetching implementation.  This strategy is more\nrobust to additions and deletions of properties.\n\nAs keep_pack is a single bit we have also introduced lock_pack to\nmean not only download and store the packfile via index-pack but\nalso to lock it against repacking by creating a .keep file when\nthe packfile itself is stored.  The caller must remove the .keep\nfile when it is safe to do so.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n builtin-fetch-pack.c |  111 +++++++++++++++++++++-----------------------------\n fetch-pack.h         |    9 +++-\n transport.c          |    9 +---\n 3 files changed, 55 insertions(+), 74 deletions(-)\n\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex 2977a94..77eb181 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -8,15 +8,11 @@\n #include \"sideband.h\"\n #include \"fetch-pack.h\"\n \n-static int keep_pack;\n static int transfer_unpack_limit = -1;\n static int fetch_unpack_limit = -1;\n static int unpack_limit = 100;\n-static int quiet;\n-static int verbose;\n-static int fetch_all;\n-static int depth;\n-static int no_progress;\n+static struct fetch_pack_args args;\n+\n static const char fetch_pack_usage[] =\n \"git-fetch-pack [--all] [--quiet|-q] [--keep|-k] [--thin] [--upload-pack=<git-upload-pack>] [--depth=<n>] [--no-progress] [-v] [<host>:]<directory> [<refs>...]\";\n static const char *uploadpack = \"git-upload-pack\";\n@@ -181,7 +177,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \t\t\t\t     (use_sideband == 2 ? \" side-band-64k\" : \"\"),\n \t\t\t\t     (use_sideband == 1 ? \" side-band\" : \"\"),\n \t\t\t\t     (use_thin_pack ? \" thin-pack\" : \"\"),\n-\t\t\t\t     (no_progress ? \" no-progress\" : \"\"),\n+\t\t\t\t     (args.no_progress ? \" no-progress\" : \"\"),\n \t\t\t\t     \" ofs-delta\");\n \t\telse\n \t\t\tpacket_write(fd[1], \"want %s\\n\", sha1_to_hex(remote));\n@@ -189,13 +185,13 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \t}\n \tif (is_repository_shallow())\n \t\twrite_shallow_commits(fd[1], 1);\n-\tif (depth > 0)\n-\t\tpacket_write(fd[1], \"deepen %d\", depth);\n+\tif (args.depth > 0)\n+\t\tpacket_write(fd[1], \"deepen %d\", args.depth);\n \tpacket_flush(fd[1]);\n \tif (!fetching)\n \t\treturn 1;\n \n-\tif (depth > 0) {\n+\tif (args.depth > 0) {\n \t\tchar line[1024];\n \t\tunsigned char sha1[20];\n \t\tint len;\n@@ -226,7 +222,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \tretval = -1;\n \twhile ((sha1 = get_rev())) {\n \t\tpacket_write(fd[1], \"have %s\\n\", sha1_to_hex(sha1));\n-\t\tif (verbose)\n+\t\tif (args.verbose)\n \t\t\tfprintf(stderr, \"have %s\\n\", sha1_to_hex(sha1));\n \t\tin_vain++;\n \t\tif (!(31 & ++count)) {\n@@ -244,7 +240,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \n \t\t\tdo {\n \t\t\t\tack = get_ack(fd[0], result_sha1);\n-\t\t\t\tif (verbose && ack)\n+\t\t\t\tif (args.verbose && ack)\n \t\t\t\t\tfprintf(stderr, \"got ack %d %s\\n\", ack,\n \t\t\t\t\t\t\tsha1_to_hex(result_sha1));\n \t\t\t\tif (ack == 1) {\n@@ -263,7 +259,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \t\t\t} while (ack);\n \t\t\tflushes--;\n \t\t\tif (got_continue && MAX_IN_VAIN < in_vain) {\n-\t\t\t\tif (verbose)\n+\t\t\t\tif (args.verbose)\n \t\t\t\t\tfprintf(stderr, \"giving up\\n\");\n \t\t\t\tbreak; /* give up */\n \t\t\t}\n@@ -271,7 +267,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \t}\n done:\n \tpacket_write(fd[1], \"done\\n\");\n-\tif (verbose)\n+\tif (args.verbose)\n \t\tfprintf(stderr, \"done\\n\");\n \tif (retval != 0) {\n \t\tmulti_ack = 0;\n@@ -280,7 +276,7 @@ done:\n \twhile (flushes || multi_ack) {\n \t\tint ack = get_ack(fd[0], result_sha1);\n \t\tif (ack) {\n-\t\t\tif (verbose)\n+\t\t\tif (args.verbose)\n \t\t\t\tfprintf(stderr, \"got ack (%d) %s\\n\", ack,\n \t\t\t\t\tsha1_to_hex(result_sha1));\n \t\t\tif (ack == 1)\n@@ -317,7 +313,7 @@ static int mark_complete(const char *path, const unsigned char *sha1, int flag,\n static void mark_recent_complete_commits(unsigned long cutoff)\n {\n \twhile (complete && cutoff <= complete->item->date) {\n-\t\tif (verbose)\n+\t\tif (args.verbose)\n \t\t\tfprintf(stderr, \"Marking %s as complete\\n\",\n \t\t\t\tsha1_to_hex(complete->item->object.sha1));\n \t\tpop_most_recent_commit(&complete, COMPLETE);\n@@ -332,7 +328,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \tstruct ref *ref, *next;\n \tstruct ref *fastarray[32];\n \n-\tif (nr_match && !fetch_all) {\n+\tif (nr_match && !args.fetch_all) {\n \t\tif (ARRAY_SIZE(fastarray) < nr_match)\n \t\t\treturn_refs = xcalloc(nr_match, sizeof(struct ref *));\n \t\telse {\n@@ -348,8 +344,8 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \t\tif (!memcmp(ref->name, \"refs/\", 5) &&\n \t\t    check_ref_format(ref->name + 5))\n \t\t\t; /* trash */\n-\t\telse if (fetch_all &&\n-\t\t\t (!depth || prefixcmp(ref->name, \"refs/tags/\") )) {\n+\t\telse if (args.fetch_all &&\n+\t\t\t (!args.depth || prefixcmp(ref->name, \"refs/tags/\") )) {\n \t\t\t*newtail = ref;\n \t\t\tref->next = NULL;\n \t\t\tnewtail = &ref->next;\n@@ -365,7 +361,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \t\tfree(ref);\n \t}\n \n-\tif (!fetch_all) {\n+\tif (!args.fetch_all) {\n \t\tint i;\n \t\tfor (i = 0; i < nr_match; i++) {\n \t\t\tref = return_refs[i];\n@@ -408,7 +404,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)\n \t\t}\n \t}\n \n-\tif (!depth) {\n+\tif (!args.depth) {\n \t\tfor_each_ref(mark_complete, NULL);\n \t\tif (cutoff)\n \t\t\tmark_recent_complete_commits(cutoff);\n@@ -442,7 +438,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)\n \t\to = lookup_object(remote);\n \t\tif (!o || !(o->flags & COMPLETE)) {\n \t\t\tretval = 0;\n-\t\t\tif (!verbose)\n+\t\t\tif (!args.verbose)\n \t\t\t\tcontinue;\n \t\t\tfprintf(stderr,\n \t\t\t\t\"want %s (%s)\\n\", sha1_to_hex(remote),\n@@ -451,7 +447,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)\n \t\t}\n \n \t\thashcpy(ref->new_sha1, local);\n-\t\tif (!verbose)\n+\t\tif (!args.verbose)\n \t\t\tcontinue;\n \t\tfprintf(stderr,\n \t\t\t\"already have %s (%s)\\n\", sha1_to_hex(remote),\n@@ -502,14 +498,14 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \tchar keep_arg[256];\n \tchar hdr_arg[256];\n \tconst char **av;\n-\tint do_keep = keep_pack;\n+\tint do_keep = args.keep_pack;\n \tint keep_pipe[2];\n \n \tside_pid = setup_sideband(fd, xd);\n \n \tav = argv;\n \t*hdr_arg = 0;\n-\tif (unpack_limit) {\n+\tif (!args.keep_pack && unpack_limit) {\n \t\tstruct pack_header header;\n \n \t\tif (read_pack_header(fd[0], &header))\n@@ -527,11 +523,11 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \t\t\tdie(\"fetch-pack: pipe setup failure: %s\", strerror(errno));\n \t\t*av++ = \"index-pack\";\n \t\t*av++ = \"--stdin\";\n-\t\tif (!quiet && !no_progress)\n+\t\tif (!args.quiet && !args.no_progress)\n \t\t\t*av++ = \"-v\";\n-\t\tif (use_thin_pack)\n+\t\tif (args.use_thin_pack)\n \t\t\t*av++ = \"--fix-thin\";\n-\t\tif (keep_pack > 1 || unpack_limit) {\n+\t\tif (args.lock_pack || unpack_limit) {\n \t\t\tint s = sprintf(keep_arg,\n \t\t\t\t\t\"--keep=fetch-pack %d on \", getpid());\n \t\t\tif (gethostname(keep_arg + s, sizeof(keep_arg) - s))\n@@ -541,7 +537,7 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \t}\n \telse {\n \t\t*av++ = \"unpack-objects\";\n-\t\tif (quiet)\n+\t\tif (args.quiet)\n \t\t\t*av++ = \"-q\";\n \t}\n \tif (*hdr_arg)\n@@ -599,17 +595,17 @@ static struct ref *do_fetch_pack(int fd[2],\n \tif (is_repository_shallow() && !server_supports(\"shallow\"))\n \t\tdie(\"Server does not support shallow clients\");\n \tif (server_supports(\"multi_ack\")) {\n-\t\tif (verbose)\n+\t\tif (args.verbose)\n \t\t\tfprintf(stderr, \"Server supports multi_ack\\n\");\n \t\tmulti_ack = 1;\n \t}\n \tif (server_supports(\"side-band-64k\")) {\n-\t\tif (verbose)\n+\t\tif (args.verbose)\n \t\t\tfprintf(stderr, \"Server supports side-band-64k\\n\");\n \t\tuse_sideband = 2;\n \t}\n \telse if (server_supports(\"side-band\")) {\n-\t\tif (verbose)\n+\t\tif (args.verbose)\n \t\t\tfprintf(stderr, \"Server supports side-band\\n\");\n \t\tuse_sideband = 1;\n \t}\n@@ -622,7 +618,7 @@ static struct ref *do_fetch_pack(int fd[2],\n \t\tgoto all_done;\n \t}\n \tif (find_common(fd, sha1, ref) < 0)\n-\t\tif (keep_pack != 1)\n+\t\tif (!args.keep_pack)\n \t\t\t/* When cloning, it is not unusual to have\n \t\t\t * no common commit.\n \t\t\t */\n@@ -674,22 +670,6 @@ static int fetch_pack_config(const char *var, const char *value)\n \n static struct lock_file lock;\n \n-void setup_fetch_pack(struct fetch_pack_args *args)\n-{\n-\tuploadpack = args->uploadpack;\n-\tquiet = args->quiet;\n-\tkeep_pack = args->keep_pack;\n-\tif (args->unpacklimit >= 0)\n-\t\tunpack_limit = args->unpacklimit;\n-\tif (args->keep_pack)\n-\t\tunpack_limit = 0;\n-\tuse_thin_pack = args->use_thin_pack;\n-\tfetch_all = args->fetch_all;\n-\tverbose = args->verbose;\n-\tdepth = args->depth;\n-\tno_progress = args->no_progress;\n-}\n-\n int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n {\n \tint i, ret, nr_heads;\n@@ -710,40 +690,40 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \n \t\tif (*arg == '-') {\n \t\t\tif (!prefixcmp(arg, \"--upload-pack=\")) {\n-\t\t\t\tuploadpack = arg + 14;\n+\t\t\t\targs.uploadpack = arg + 14;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!prefixcmp(arg, \"--exec=\")) {\n-\t\t\t\tuploadpack = arg + 7;\n+\t\t\t\targs.uploadpack = arg + 7;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"--quiet\", arg) || !strcmp(\"-q\", arg)) {\n-\t\t\t\tquiet = 1;\n+\t\t\t\targs.quiet = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"--keep\", arg) || !strcmp(\"-k\", arg)) {\n-\t\t\t\tkeep_pack++;\n-\t\t\t\tunpack_limit = 0;\n+\t\t\t\targs.lock_pack = args.keep_pack;\n+\t\t\t\targs.keep_pack = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"--thin\", arg)) {\n-\t\t\t\tuse_thin_pack = 1;\n+\t\t\t\targs.use_thin_pack = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"--all\", arg)) {\n-\t\t\t\tfetch_all = 1;\n+\t\t\t\targs.fetch_all = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"-v\", arg)) {\n-\t\t\t\tverbose = 1;\n+\t\t\t\targs.verbose = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!prefixcmp(arg, \"--depth=\")) {\n-\t\t\t\tdepth = strtol(arg + 8, NULL, 0);\n+\t\t\t\targs.depth = strtol(arg + 8, NULL, 0);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"--no-progress\", arg)) {\n-\t\t\t\tno_progress = 1;\n+\t\t\t\targs.no_progress = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tusage(fetch_pack_usage);\n@@ -756,8 +736,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tif (!dest)\n \t\tusage(fetch_pack_usage);\n \n-\tref = fetch_pack(dest, nr_heads, heads, NULL);\n-\n+\tref = fetch_pack(&args, dest, nr_heads, heads, NULL);\n \tret = !ref;\n \n \twhile (ref) {\n@@ -769,7 +748,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \treturn ret;\n }\n \n-struct ref *fetch_pack(const char *dest,\n+struct ref *fetch_pack(struct fetch_pack_args *my_args,\n+\t\tconst char *dest,\n \t\tint nr_heads,\n \t\tchar **heads,\n \t\tchar **pack_lockfile)\n@@ -780,13 +760,14 @@ struct ref *fetch_pack(const char *dest,\n \tstruct ref *ref;\n \tstruct stat st;\n \n-\tif (depth > 0) {\n+\tmemcpy(&args, my_args, sizeof(args));\n+\tif (args.depth > 0) {\n \t\tif (stat(git_path(\"shallow\"), &st))\n \t\t\tst.st_mtime = 0;\n \t}\n \n \tpid = git_connect(fd, (char *)dest, uploadpack,\n-                          verbose ? CONNECT_VERBOSE : 0);\n+                          args.verbose ? CONNECT_VERBOSE : 0);\n \tif (pid < 0)\n \t\treturn NULL;\n \tif (heads && nr_heads)\n@@ -809,7 +790,7 @@ struct ref *fetch_pack(const char *dest,\n \t\t\t}\n \t}\n \n-\tif (!ret && depth > 0) {\n+\tif (!ret && args.depth > 0) {\n \t\tstruct cache_time mtime;\n \t\tchar *shallow = git_path(\"shallow\");\n \t\tint fd;\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex ad13076..a7888ea 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -8,14 +8,17 @@ struct fetch_pack_args\n \tint depth;\n \tunsigned quiet:1,\n \t\tkeep_pack:1,\n+\t\tlock_pack:1,\n \t\tuse_thin_pack:1,\n \t\tfetch_all:1,\n \t\tverbose:1,\n \t\tno_progress:1;\n };\n \n-void setup_fetch_pack(struct fetch_pack_args *args);\n-\n-struct ref *fetch_pack(const char *dest, int nr_heads, char **heads, char **pack_lockfile);\n+struct ref *fetch_pack(struct fetch_pack_args *args,\n+\t\tconst char *dest,\n+\t\tint nr_heads,\n+\t\tchar **heads,\n+\t\tchar **pack_lockfile);\n \n #endif\ndiff --git a/transport.c b/transport.c\nindex d8458dc..85f5b1e 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -314,21 +314,18 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tstruct fetch_pack_args args;\n \tint i;\n \n+\tmemset(&args, 0, sizeof(args));\n \targs.uploadpack = data->uploadpack;\n-\targs.quiet = 0;\n \targs.keep_pack = data->keep;\n+\targs.lock_pack = 1;\n \targs.unpacklimit = data->unpacklimit;\n \targs.use_thin_pack = data->thin;\n-\targs.fetch_all = 0;\n \targs.verbose = transport->verbose;\n \targs.depth = data->depth;\n-\targs.no_progress = 0;\n-\n-\tsetup_fetch_pack(&args);\n \n \tfor (i = 0; i < nr_heads; i++)\n \t\torigh[i] = heads[i] = xstrdup(to_fetch[i]->name);\n-\trefs = fetch_pack(dest, nr_heads, heads, &transport->pack_lockfile);\n+\trefs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile);\n \n \tfor (i = 0; i < nr_heads; i++)\n \t\tfree(origh[i]);\n-- \n1.5.3.1.195.gadd6\n"}]}