{"thread":{"id":"18760","subject":"[PATCH 2/3] rev-list: remove last static vars used in \"show_commit\"","startedAt":"2009-04-06T19:28:36Z","lastAt":"2009-04-09T03:11:08Z","messageCount":8,"participants":["Christian Couder","Junio C Hamano","Paolo Bonzini"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"110652","messageId":"20090407040854.4338.94304.chriscool@tuxfamily.org","threadId":"18760","inReplyTo":"20090407040819.4338.4291.chriscool@tuxfamily.org","subject":"[PATCH 1/3] list-objects: add \"void *data\" parameter to show functions","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-06T19:28:36Z","receivedAt":"2009-04-06T19:28:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The goal of this patch is to get rid of the \"static struct rev_info\nrevs\" static variable in \"builtin-rev-list.c\".\n\nTo do that, we need to pass the revs to the \"show_commit\" function\nin \"builtin-rev-list.c\" and this in turn means that the\n\"traverse_commit_list\" function in \"list-objects.c\" must be passed\nfunctions pointers to functions with 2 parameters instead of one.\n\nSo we have to change all the callers and all the functions passed\nto \"traverse_commit_list\".\n\nAnyway this makes the code more clean and more generic, so it\nshould be a good thing in the long run.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-pack-objects.c |    6 ++--\n builtin-rev-list.c     |   68 ++++++++++++++++++++++++-----------------------\n list-objects.c         |    9 +++---\n list-objects.h         |    6 ++--\n upload-pack.c          |    6 ++--\n 5 files changed, 49 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 50f8cc1..8c5eaba 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1901,13 +1901,13 @@ static void read_object_list_from_stdin(void)\n \n #define OBJECT_ADDED (1u<<20)\n \n-static void show_commit(struct commit *commit)\n+static void show_commit(struct commit *commit, void *data)\n {\n \tadd_object_entry(commit->object.sha1, OBJ_COMMIT, NULL, 0);\n \tcommit->object.flags |= OBJECT_ADDED;\n }\n \n-static void show_object(struct object_array_entry *p)\n+static void show_object(struct object_array_entry *p, void *data)\n {\n \tadd_preferred_base_object(p->name);\n \tadd_object_entry(p->item->sha1, p->item->type, p->name, 0);\n@@ -2071,7 +2071,7 @@ static void get_object_list(int ac, const char **av)\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n-\ttraverse_commit_list(&revs, show_commit, show_object);\n+\ttraverse_commit_list(&revs, show_commit, show_object, NULL);\n \n \tif (keep_unreachable)\n \t\tadd_objects_in_unpacked_packs(&revs);\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex eb34147..cd6f6b8 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -42,72 +42,72 @@ static const char rev_list_usage[] =\n \"    --bisect-all\"\n ;\n \n-static struct rev_info revs;\n-\n static int show_timestamp;\n static int hdr_termination;\n static const char *header_prefix;\n \n-static void finish_commit(struct commit *commit);\n-static void show_commit(struct commit *commit)\n+static void finish_commit(struct commit *commit, void *data);\n+static void show_commit(struct commit *commit, void *data)\n {\n-\tgraph_show_commit(revs.graph);\n+\tstruct rev_info *revs = data;\n+\n+\tgraph_show_commit(revs->graph);\n \n \tif (show_timestamp)\n \t\tprintf(\"%lu \", commit->date);\n \tif (header_prefix)\n \t\tfputs(header_prefix, stdout);\n \n-\tif (!revs.graph) {\n+\tif (!revs->graph) {\n \t\tif (commit->object.flags & BOUNDARY)\n \t\t\tputchar('-');\n \t\telse if (commit->object.flags & UNINTERESTING)\n \t\t\tputchar('^');\n-\t\telse if (revs.left_right) {\n+\t\telse if (revs->left_right) {\n \t\t\tif (commit->object.flags & SYMMETRIC_LEFT)\n \t\t\t\tputchar('<');\n \t\t\telse\n \t\t\t\tputchar('>');\n \t\t}\n \t}\n-\tif (revs.abbrev_commit && revs.abbrev)\n-\t\tfputs(find_unique_abbrev(commit->object.sha1, revs.abbrev),\n+\tif (revs->abbrev_commit && revs->abbrev)\n+\t\tfputs(find_unique_abbrev(commit->object.sha1, revs->abbrev),\n \t\t      stdout);\n \telse\n \t\tfputs(sha1_to_hex(commit->object.sha1), stdout);\n-\tif (revs.print_parents) {\n+\tif (revs->print_parents) {\n \t\tstruct commit_list *parents = commit->parents;\n \t\twhile (parents) {\n \t\t\tprintf(\" %s\", sha1_to_hex(parents->item->object.sha1));\n \t\t\tparents = parents->next;\n \t\t}\n \t}\n-\tif (revs.children.name) {\n+\tif (revs->children.name) {\n \t\tstruct commit_list *children;\n \n-\t\tchildren = lookup_decoration(&revs.children, &commit->object);\n+\t\tchildren = lookup_decoration(&revs->children, &commit->object);\n \t\twhile (children) {\n \t\t\tprintf(\" %s\", sha1_to_hex(children->item->object.sha1));\n \t\t\tchildren = children->next;\n \t\t}\n \t}\n-\tshow_decorations(&revs, commit);\n-\tif (revs.commit_format == CMIT_FMT_ONELINE)\n+\tshow_decorations(revs, commit);\n+\tif (revs->commit_format == CMIT_FMT_ONELINE)\n \t\tputchar(' ');\n \telse\n \t\tputchar('\\n');\n \n-\tif (revs.verbose_header && commit->buffer) {\n+\tif (revs->verbose_header && commit->buffer) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tpretty_print_commit(revs.commit_format, commit,\n-\t\t\t\t    &buf, revs.abbrev, NULL, NULL,\n-\t\t\t\t    revs.date_mode, 0);\n-\t\tif (revs.graph) {\n+\t\tpretty_print_commit(revs->commit_format, commit,\n+\t\t\t\t    &buf, revs->abbrev, NULL, NULL,\n+\t\t\t\t    revs->date_mode, 0);\n+\t\tif (revs->graph) {\n \t\t\tif (buf.len) {\n-\t\t\t\tif (revs.commit_format != CMIT_FMT_ONELINE)\n-\t\t\t\t\tgraph_show_oneline(revs.graph);\n+\t\t\t\tif (revs->commit_format != CMIT_FMT_ONELINE)\n+\t\t\t\t\tgraph_show_oneline(revs->graph);\n \n-\t\t\t\tgraph_show_commit_msg(revs.graph, &buf);\n+\t\t\t\tgraph_show_commit_msg(revs->graph, &buf);\n \n \t\t\t\t/*\n \t\t\t\t * Add a newline after the commit message.\n@@ -125,7 +125,7 @@ static void show_commit(struct commit *commit)\n \t\t\t\t * format doesn't explicitly end in a newline.)\n \t\t\t\t */\n \t\t\t\tif (buf.len && buf.buf[buf.len - 1] == '\\n')\n-\t\t\t\t\tgraph_show_padding(revs.graph);\n+\t\t\t\t\tgraph_show_padding(revs->graph);\n \t\t\t\tputchar('\\n');\n \t\t\t} else {\n \t\t\t\t/*\n@@ -133,7 +133,7 @@ static void show_commit(struct commit *commit)\n \t\t\t\t * the rest of the graph output for this\n \t\t\t\t * commit.\n \t\t\t\t */\n-\t\t\t\tif (graph_show_remainder(revs.graph))\n+\t\t\t\tif (graph_show_remainder(revs->graph))\n \t\t\t\t\tputchar('\\n');\n \t\t\t}\n \t\t} else {\n@@ -142,14 +142,14 @@ static void show_commit(struct commit *commit)\n \t\t}\n \t\tstrbuf_release(&buf);\n \t} else {\n-\t\tif (graph_show_remainder(revs.graph))\n+\t\tif (graph_show_remainder(revs->graph))\n \t\t\tputchar('\\n');\n \t}\n \tmaybe_flush_or_die(stdout, \"stdout\");\n-\tfinish_commit(commit);\n+\tfinish_commit(commit, data);\n }\n \n-static void finish_commit(struct commit *commit)\n+static void finish_commit(struct commit *commit, void *data)\n {\n \tif (commit->parents) {\n \t\tfree_commit_list(commit->parents);\n@@ -159,20 +159,20 @@ static void finish_commit(struct commit *commit)\n \tcommit->buffer = NULL;\n }\n \n-static void finish_object(struct object_array_entry *p)\n+static void finish_object(struct object_array_entry *p, void *data)\n {\n \tif (p->item->type == OBJ_BLOB && !has_sha1_file(p->item->sha1))\n \t\tdie(\"missing blob object '%s'\", sha1_to_hex(p->item->sha1));\n }\n \n-static void show_object(struct object_array_entry *p)\n+static void show_object(struct object_array_entry *p, void *data)\n {\n \t/* An object with name \"foo\\n0000000...\" can be used to\n \t * confuse downstream \"git pack-objects\" very badly.\n \t */\n \tconst char *ep = strchr(p->name, '\\n');\n \n-\tfinish_object(p);\n+\tfinish_object(p, data);\n \tif (ep) {\n \t\tprintf(\"%s %.*s\\n\", sha1_to_hex(p->item->sha1),\n \t\t       (int) (ep - p->name),\n@@ -264,7 +264,7 @@ int show_bisect_vars(struct rev_info *revs, int reaches, int all, int flags)\n \t\tstrcpy(hex, sha1_to_hex(revs->commits->item->object.sha1));\n \n \tif (flags & BISECT_SHOW_ALL) {\n-\t\ttraverse_commit_list(revs, show_commit, show_object);\n+\t\ttraverse_commit_list(revs, show_commit, show_object, revs);\n \t\tprintf(\"------\\n\");\n \t}\n \n@@ -297,6 +297,7 @@ int show_bisect_vars(struct rev_info *revs, int reaches, int all, int flags)\n \n int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n+\tstruct rev_info revs;\n \tstruct commit_list *list;\n \tint i;\n \tint read_from_stdin = 0;\n@@ -391,8 +392,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t}\n \n \ttraverse_commit_list(&revs,\n-\t\tquiet ? finish_commit : show_commit,\n-\t\tquiet ? finish_object : show_object);\n+\t\t\t     quiet ? finish_commit : show_commit,\n+\t\t\t     quiet ? finish_object : show_object,\n+\t\t\t     &revs);\n \n \treturn 0;\n }\ndiff --git a/list-objects.c b/list-objects.c\nindex c8b8375..208a4cb 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -137,8 +137,9 @@ void mark_edges_uninteresting(struct commit_list *list,\n }\n \n void traverse_commit_list(struct rev_info *revs,\n-\t\t\t  void (*show_commit)(struct commit *),\n-\t\t\t  void (*show_object)(struct object_array_entry *))\n+\t\t\t  show_commit_fn show_commit,\n+\t\t\t  show_object_fn show_object,\n+\t\t\t  void *data)\n {\n \tint i;\n \tstruct commit *commit;\n@@ -146,7 +147,7 @@ void traverse_commit_list(struct rev_info *revs,\n \n \twhile ((commit = get_revision(revs)) != NULL) {\n \t\tprocess_tree(revs, commit->tree, &objects, NULL, \"\");\n-\t\tshow_commit(commit);\n+\t\tshow_commit(commit, data);\n \t}\n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n@@ -173,7 +174,7 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t    sha1_to_hex(obj->sha1), name);\n \t}\n \tfor (i = 0; i < objects.nr; i++)\n-\t\tshow_object(&objects.objects[i]);\n+\t\tshow_object(&objects.objects[i], data);\n \tfree(objects.objects);\n \tif (revs->pending.nr) {\n \t\tfree(revs->pending.objects);\ndiff --git a/list-objects.h b/list-objects.h\nindex 0f41391..47fae2e 100644\n--- a/list-objects.h\n+++ b/list-objects.h\n@@ -1,11 +1,11 @@\n #ifndef LIST_OBJECTS_H\n #define LIST_OBJECTS_H\n \n-typedef void (*show_commit_fn)(struct commit *);\n-typedef void (*show_object_fn)(struct object_array_entry *);\n+typedef void (*show_commit_fn)(struct commit *, void *);\n+typedef void (*show_object_fn)(struct object_array_entry *, void *);\n typedef void (*show_edge_fn)(struct commit *);\n \n-void traverse_commit_list(struct rev_info *revs, show_commit_fn, show_object_fn);\n+void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);\n \n void mark_edges_uninteresting(struct commit_list *, struct rev_info *, show_edge_fn);\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex 4d7357f..a28a0c4 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -66,7 +66,7 @@ static ssize_t send_client_data(int fd, const char *data, ssize_t sz)\n }\n \n static FILE *pack_pipe = NULL;\n-static void show_commit(struct commit *commit)\n+static void show_commit(struct commit *commit, void *data)\n {\n \tif (commit->object.flags & BOUNDARY)\n \t\tfputc('-', pack_pipe);\n@@ -78,7 +78,7 @@ static void show_commit(struct commit *commit)\n \tcommit->buffer = NULL;\n }\n \n-static void show_object(struct object_array_entry *p)\n+static void show_object(struct object_array_entry *p, void *data)\n {\n \t/* An object with name \"foo\\n0000000...\" can be used to\n \t * confuse downstream git-pack-objects very badly.\n@@ -134,7 +134,7 @@ static int do_rev_list(int fd, void *create_full_pack)\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n-\ttraverse_commit_list(&revs, show_commit, show_object);\n+\ttraverse_commit_list(&revs, show_commit, show_object, NULL);\n \tfflush(pack_pipe);\n \tfclose(pack_pipe);\n \treturn 0;\n-- \n1.6.2.2.537.g3b83b\n"},{"id":"110651","messageId":"20090407040854.4338.40055.chriscool@tuxfamily.org","threadId":"18760","inReplyTo":"20090407040854.4338.94304.chriscool@tuxfamily.org","subject":"[PATCH 2/3] rev-list: remove last static vars used in \"show_commit\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-06T20:28:00Z","receivedAt":"2009-04-06T20:28:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch removes the last static variables that were used in\nthe \"show_commit\" function.\n\nTo do that, we create a new \"rev_list_info\" struct that we will pass\nin the \"void *data\" argument to \"show_commit\".\n\nThis means that we have to change the first argument to\n\"show_bisect_vars\" too.\n\nWhile at it, we also remove a \"struct commit_list *list\" variable\nin \"cmd_rev_list\" that is not really needed.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c           |    6 +++++-\n bisect.h           |   14 ++++++++------\n builtin-rev-list.c |   42 +++++++++++++++++++++---------------------\n 3 files changed, 34 insertions(+), 28 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 69f8860..4d2a150 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -535,8 +535,12 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix)\n int bisect_next_vars(const char *prefix)\n {\n \tstruct rev_info revs;\n+\tstruct rev_list_info info;\n \tint reaches = 0, all = 0;\n \n+\tmemset(&info, 0, sizeof(info));\n+\tinfo.revs = &revs;\n+\n \tbisect_rev_setup(&revs, prefix);\n \n \tif (prepare_revision_walk(&revs))\n@@ -547,6 +551,6 @@ int bisect_next_vars(const char *prefix)\n \trevs.commits = find_bisection(revs.commits, &reaches, &all,\n \t\t\t\t      !!skipped_sha1_nr);\n \n-\treturn show_bisect_vars(&revs, reaches, all,\n+\treturn show_bisect_vars(&info, reaches, all,\n \t\t\t\tBISECT_SHOW_TRIED | BISECT_SHOW_STRINGED);\n }\ndiff --git a/bisect.h b/bisect.h\nindex f5d1067..b1c334d 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -14,12 +14,14 @@ extern struct commit_list *filter_skipped(struct commit_list *list,\n #define BISECT_SHOW_TRIED\t(1<<1)\n #define BISECT_SHOW_STRINGED\t(1<<2)\n \n-/*\n- * The flag BISECT_SHOW_ALL should not be set if this function is called\n- * from outside \"builtin-rev-list.c\" as otherwise it would use\n- * static \"revs\" from this file.\n- */\n-extern int show_bisect_vars(struct rev_info *revs, int reaches, int all,\n+struct rev_list_info {\n+\tstruct rev_info *revs;\n+\tint show_timestamp;\n+\tint hdr_termination;\n+\tconst char *header_prefix;\n+};\n+\n+extern int show_bisect_vars(struct rev_list_info *info, int reaches, int all,\n \t\t\t    int flags);\n \n extern int bisect_next_vars(const char *prefix);\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex cd6f6b8..244b73e 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -42,21 +42,18 @@ static const char rev_list_usage[] =\n \"    --bisect-all\"\n ;\n \n-static int show_timestamp;\n-static int hdr_termination;\n-static const char *header_prefix;\n-\n static void finish_commit(struct commit *commit, void *data);\n static void show_commit(struct commit *commit, void *data)\n {\n-\tstruct rev_info *revs = data;\n+\tstruct rev_list_info *info = data;\n+\tstruct rev_info *revs = info->revs;\n \n \tgraph_show_commit(revs->graph);\n \n-\tif (show_timestamp)\n+\tif (info->show_timestamp)\n \t\tprintf(\"%lu \", commit->date);\n-\tif (header_prefix)\n-\t\tfputs(header_prefix, stdout);\n+\tif (info->header_prefix)\n+\t\tfputs(info->header_prefix, stdout);\n \n \tif (!revs->graph) {\n \t\tif (commit->object.flags & BOUNDARY)\n@@ -138,7 +135,7 @@ static void show_commit(struct commit *commit, void *data)\n \t\t\t}\n \t\t} else {\n \t\t\tif (buf.len)\n-\t\t\t\tprintf(\"%s%c\", buf.buf, hdr_termination);\n+\t\t\t\tprintf(\"%s%c\", buf.buf, info->hdr_termination);\n \t\t}\n \t\tstrbuf_release(&buf);\n \t} else {\n@@ -236,11 +233,13 @@ static void show_tried_revs(struct commit_list *tried, int stringed)\n \tprintf(stringed ? \"' &&\\n\" : \"'\\n\");\n }\n \n-int show_bisect_vars(struct rev_info *revs, int reaches, int all, int flags)\n+int show_bisect_vars(struct rev_list_info *info, int reaches, int all,\n+\t\t     int flags)\n {\n \tint cnt;\n \tchar hex[41] = \"\", *format;\n \tstruct commit_list *tried;\n+\tstruct rev_info *revs = info->revs;\n \n \tif (!revs->commits && !(flags & BISECT_SHOW_TRIED))\n \t\treturn 1;\n@@ -264,7 +263,7 @@ int show_bisect_vars(struct rev_info *revs, int reaches, int all, int flags)\n \t\tstrcpy(hex, sha1_to_hex(revs->commits->item->object.sha1));\n \n \tif (flags & BISECT_SHOW_ALL) {\n-\t\ttraverse_commit_list(revs, show_commit, show_object, revs);\n+\t\ttraverse_commit_list(revs, show_commit, show_object, info);\n \t\tprintf(\"------\\n\");\n \t}\n \n@@ -298,7 +297,7 @@ int show_bisect_vars(struct rev_info *revs, int reaches, int all, int flags)\n int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n-\tstruct commit_list *list;\n+\tstruct rev_list_info info;\n \tint i;\n \tint read_from_stdin = 0;\n \tint bisect_list = 0;\n@@ -313,6 +312,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \trevs.commit_format = CMIT_FMT_UNSPECIFIED;\n \targc = setup_revisions(argc, argv, &revs, NULL);\n \n+\tmemset(&info, 0, sizeof(info));\n+\tinfo.revs = &revs;\n+\n \tquiet = DIFF_OPT_TST(&revs.diffopt, QUIET);\n \tfor (i = 1 ; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -322,7 +324,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--timestamp\")) {\n-\t\t\tshow_timestamp = 1;\n+\t\t\tinfo.show_timestamp = 1;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--bisect\")) {\n@@ -352,19 +354,17 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t}\n \tif (revs.commit_format != CMIT_FMT_UNSPECIFIED) {\n \t\t/* The command line has a --pretty  */\n-\t\thdr_termination = '\\n';\n+\t\tinfo.hdr_termination = '\\n';\n \t\tif (revs.commit_format == CMIT_FMT_ONELINE)\n-\t\t\theader_prefix = \"\";\n+\t\t\tinfo.header_prefix = \"\";\n \t\telse\n-\t\t\theader_prefix = \"commit \";\n+\t\t\tinfo.header_prefix = \"commit \";\n \t}\n \telse if (revs.verbose_header)\n \t\t/* Only --header was specified */\n \t\trevs.commit_format = CMIT_FMT_RAW;\n \n-\tlist = revs.commits;\n-\n-\tif ((!list &&\n+\tif ((!revs.commits &&\n \t     (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) &&\n \t      !revs.pending.nr)) ||\n \t    revs.diff)\n@@ -387,14 +387,14 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t      bisect_find_all);\n \n \t\tif (bisect_show_vars)\n-\t\t\treturn show_bisect_vars(&revs, reaches, all,\n+\t\t\treturn show_bisect_vars(&info, reaches, all,\n \t\t\t\t\t\tbisect_show_all ? BISECT_SHOW_ALL : 0);\n \t}\n \n \ttraverse_commit_list(&revs,\n \t\t\t     quiet ? finish_commit : show_commit,\n \t\t\t     quiet ? finish_object : show_object,\n-\t\t\t     &revs);\n+\t\t\t     &info);\n \n \treturn 0;\n }\n-- \n1.6.2.2.537.g3b83b\n"},{"id":"110653","messageId":"20090407040854.4338.21694.chriscool@tuxfamily.org","threadId":"18760","inReplyTo":"20090407040854.4338.40055.chriscool@tuxfamily.org","subject":"[PATCH 3/3] rev-list: add \"int bisect_show_flags\" in \"struct rev_list_info\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-07T03:08:42Z","receivedAt":"2009-04-07T03:08:42Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is a cleanup patch to make it easier to use the\n\"show_bisect_vars\" function and take advantage of the rev_list_info\nstruct.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c           |    4 ++--\n bisect.h           |    6 +++---\n builtin-rev-list.c |   11 ++++-------\n 3 files changed, 9 insertions(+), 12 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 4d2a150..58f7e6f 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -540,6 +540,7 @@ int bisect_next_vars(const char *prefix)\n \n \tmemset(&info, 0, sizeof(info));\n \tinfo.revs = &revs;\n+\tinfo.bisect_show_flags = BISECT_SHOW_TRIED | BISECT_SHOW_STRINGED;\n \n \tbisect_rev_setup(&revs, prefix);\n \n@@ -551,6 +552,5 @@ int bisect_next_vars(const char *prefix)\n \trevs.commits = find_bisection(revs.commits, &reaches, &all,\n \t\t\t\t      !!skipped_sha1_nr);\n \n-\treturn show_bisect_vars(&info, reaches, all,\n-\t\t\t\tBISECT_SHOW_TRIED | BISECT_SHOW_STRINGED);\n+\treturn show_bisect_vars(&info, reaches, all);\n }\ndiff --git a/bisect.h b/bisect.h\nindex b1c334d..fdba913 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -9,20 +9,20 @@ extern struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t\t  struct commit_list **tried,\n \t\t\t\t\t  int show_all);\n \n-/* show_bisect_vars flags */\n+/* bisect_show_flags flags in struct rev_list_info */\n #define BISECT_SHOW_ALL\t\t(1<<0)\n #define BISECT_SHOW_TRIED\t(1<<1)\n #define BISECT_SHOW_STRINGED\t(1<<2)\n \n struct rev_list_info {\n \tstruct rev_info *revs;\n+\tint bisect_show_flags;\n \tint show_timestamp;\n \tint hdr_termination;\n \tconst char *header_prefix;\n };\n \n-extern int show_bisect_vars(struct rev_list_info *info, int reaches, int all,\n-\t\t\t    int flags);\n+extern int show_bisect_vars(struct rev_list_info *info, int reaches, int all);\n \n extern int bisect_next_vars(const char *prefix);\n \ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 244b73e..193993c 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -233,10 +233,9 @@ static void show_tried_revs(struct commit_list *tried, int stringed)\n \tprintf(stringed ? \"' &&\\n\" : \"'\\n\");\n }\n \n-int show_bisect_vars(struct rev_list_info *info, int reaches, int all,\n-\t\t     int flags)\n+int show_bisect_vars(struct rev_list_info *info, int reaches, int all)\n {\n-\tint cnt;\n+\tint cnt, flags = info->bisect_show_flags;\n \tchar hex[41] = \"\", *format;\n \tstruct commit_list *tried;\n \tstruct rev_info *revs = info->revs;\n@@ -303,7 +302,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tint bisect_list = 0;\n \tint bisect_show_vars = 0;\n \tint bisect_find_all = 0;\n-\tint bisect_show_all = 0;\n \tint quiet = 0;\n \n \tgit_config(git_default_config, NULL);\n@@ -334,7 +332,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(arg, \"--bisect-all\")) {\n \t\t\tbisect_list = 1;\n \t\t\tbisect_find_all = 1;\n-\t\t\tbisect_show_all = 1;\n+\t\t\tinfo.bisect_show_flags = BISECT_SHOW_ALL;\n \t\t\trevs.show_decorations = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -387,8 +385,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t      bisect_find_all);\n \n \t\tif (bisect_show_vars)\n-\t\t\treturn show_bisect_vars(&info, reaches, all,\n-\t\t\t\t\t\tbisect_show_all ? BISECT_SHOW_ALL : 0);\n+\t\t\treturn show_bisect_vars(&info, reaches, all);\n \t}\n \n \ttraverse_commit_list(&revs,\n-- \n1.6.2.2.537.g3b83b\n"},{"id":"110772","messageId":"7vk55vubcv.fsf@gitster.siamese.dyndns.org","threadId":"18760","inReplyTo":"20090407040854.4338.94304.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/3] list-objects: add \"void *data\" parameter to show functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T02:29:20Z","receivedAt":"2009-04-08T02:29:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> The goal of this patch is to get rid of the \"static struct rev_info\n> revs\" static variable in \"builtin-rev-list.c\".\n\nHmm.  If it were a more library-ish file, a removal of such a static\nvariable might help you to make more than one calls to a library function,\nbut does it matter in files like builtin-rev-list.c?  Its cmd_rev_list()\nis like main() --- it is meant to run once and exit.\n\nSo if it is the only goal of this series, I am inclined to say that I do\nnot have a reason to look at the rest of the series, but as a side effect\ndoes this removal make some other API better?  Perhaps a more library-ish\nfunction is in builtin-rev-list.c and this structure should really needs\nto be passed around as a parameter, but I cannot tell solely by reading\nthe goal above, without reading the patches themselves.\n\n> Anyway this makes the code more clean and more generic, so it\n> should be a good thing in the long run.\n\nI wouldn't disagree with that \"long run\" thing, but the answer to the\nabove question affects the placement of this series in my prioritized\nqueue.\n"},{"id":"110780","messageId":"200904080639.35499.chriscool@tuxfamily.org","threadId":"18760","inReplyTo":"7vk55vubcv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] list-objects: add \"void *data\" parameter to show functions","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-08T04:39:35Z","receivedAt":"2009-04-08T04:39:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le mercredi 8 avril 2009, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > The goal of this patch is to get rid of the \"static struct rev_info\n> > revs\" static variable in \"builtin-rev-list.c\".\n>\n> Hmm.  If it were a more library-ish file, a removal of such a static\n> variable might help you to make more than one calls to a library\n> function, but does it matter in files like builtin-rev-list.c?  Its\n> cmd_rev_list() is like main() --- it is meant to run once and exit.\n>\n> So if it is the only goal of this series, I am inclined to say that I do\n> not have a reason to look at the rest of the series, but as a side effect\n> does this removal make some other API better?  Perhaps a more library-ish\n> function is in builtin-rev-list.c and this structure should really needs\n> to be passed around as a parameter, but I cannot tell solely by reading\n> the goal above, without reading the patches themselves.\n\nIn the cover letter, I wrote that the patch series removes restrictions on \nusing the \"show_bisect_vars\" function.\n\nIn fact there was a restriction on the use of the BISECT_SHOW_ALL flag \nbecause that would use the \"show_commit\" function that was using static \nvariables.\n\nThe restriction was described in a comment in \"bisect.h\" and this comment is \nremoved by the series. \n\nThis is the relevant hunk in patch 2/3:\n\ndiff --git a/bisect.h b/bisect.h\nindex f5d1067..b1c334d 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -14,12 +14,14 @@ extern struct commit_list *filter_skipped(struct \ncommit_list *list,\n #define BISECT_SHOW_TRIED      (1<<1)\n #define BISECT_SHOW_STRINGED   (1<<2)\n \n-/*\n- * The flag BISECT_SHOW_ALL should not be set if this function is called\n- * from outside \"builtin-rev-list.c\" as otherwise it would use\n- * static \"revs\" from this file.\n- */\n-extern int show_bisect_vars(struct rev_info *revs, int reaches, int all,\n+struct rev_list_info {\n+       struct rev_info *revs;\n+       int show_timestamp;\n+       int hdr_termination;\n+       const char *header_prefix;\n+};\n+\n+extern int show_bisect_vars(struct rev_list_info *info, int reaches, int \nall,\n                            int flags);\n \n extern int bisect_next_vars(const char *prefix);\n\n\nBest regards,\nChristian.\n\n> > Anyway this makes the code more clean and more generic, so it\n> > should be a good thing in the long run.\n>\n> I wouldn't disagree with that \"long run\" thing, but the answer to the\n> above question affects the placement of this series in my prioritized\n> queue.\n"},{"id":"110784","messageId":"7vk55vspbx.fsf@gitster.siamese.dyndns.org","threadId":"18760","inReplyTo":"200904080639.35499.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/3] list-objects: add \"void *data\" parameter to show functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T05:10:26Z","receivedAt":"2009-04-08T05:10:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> This is the relevant hunk in patch 2/3:\n>\n> diff --git a/bisect.h b/bisect.h\n> index f5d1067..b1c334d 100644\n> --- a/bisect.h\n> +++ b/bisect.h\n> @@ -14,12 +14,14 @@ extern struct commit_list *filter_skipped(struct \n> commit_list *list,\n>  #define BISECT_SHOW_TRIED      (1<<1)\n>  #define BISECT_SHOW_STRINGED   (1<<2)\n>  \n> -/*\n> - * The flag BISECT_SHOW_ALL should not be set if this function is called\n> - * from outside \"builtin-rev-list.c\" as otherwise it would use\n> - * static \"revs\" from this file.\n> - */\n\nThanks for a clarification.\n"},{"id":"110806","messageId":"49DC54DC.3060408@gnu.org","threadId":"18760","inReplyTo":"20090407040854.4338.40055.chriscool@tuxfamily.org","subject":"Re: [PATCH 2/3] rev-list: remove last static vars used in \"show_commit\"","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-04-08T07:40:12Z","receivedAt":"2009-04-08T07:40:12Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"\n>  \tstruct rev_info revs;\n> +\tstruct rev_list_info info;\n>  \tint reaches = 0, all = 0;\n>  \n> +\tmemset(&info, 0, sizeof(info));\n> +\tinfo.revs = &revs;\n\nWould it make sense to embed the struct rev_info entirely in the new\nstruct, without going through a pointer?\n\nPaolo\n"},{"id":"110886","messageId":"200904090511.08242.chriscool@tuxfamily.org","threadId":"18760","inReplyTo":"49DC54DC.3060408@gnu.org","subject":"Re: [PATCH 2/3] rev-list: remove last static vars used in \"show_commit\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-09T03:11:08Z","receivedAt":"2009-04-09T03:11:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le mercredi 8 avril 2009, Paolo Bonzini a écrit :\n> >  \tstruct rev_info revs;\n> > +\tstruct rev_list_info info;\n> >  \tint reaches = 0, all = 0;\n> >\n> > +\tmemset(&info, 0, sizeof(info));\n> > +\tinfo.revs = &revs;\n>\n> Would it make sense to embed the struct rev_info entirely in the new\n> struct, without going through a pointer?\n\nPerhaps, I will have a look.\n\nThe downside is that the struct rev_info may be initialized twice if we \nstill use \"memset(&info, 0, sizeof(info))\" as it is also initialized \nin \"init_revisions\".\n\nThanks,\nChristian.\n"}]}