{"thread":{"id":"57401","subject":"[PATCH v2 1/4] log: fix memory leak if --graph is passed multiple times","startedAt":"2022-02-10T16:53:10Z","lastAt":"2022-02-10T16:53:21Z","messageCount":4,"participants":["Alex Henrie"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"448167","messageId":"20220210165209.310930-1-alexhenrie24@gmail.com","threadId":"57401","inReplyTo":null,"subject":"[PATCH v2 1/4] log: fix memory leak if --graph is passed multiple times","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-02-10T16:52:06Z","receivedAt":"2022-02-10T16:53:10Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\nv2: introduce graph_clear function\n---\n graph.c    | 12 ++++++++++++\n graph.h    |  5 +++++\n revision.c |  1 +\n 3 files changed, 18 insertions(+)\n\ndiff --git a/graph.c b/graph.c\nindex e3828eb8f2..568b6e7cd4 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -401,6 +401,18 @@ struct git_graph *graph_init(struct rev_info *opt)\n \treturn graph;\n }\n \n+void graph_clear(struct git_graph *graph)\n+{\n+\tif (!graph)\n+\t\treturn;\n+\n+\tfree(graph->columns);\n+\tfree(graph->new_columns);\n+\tfree(graph->mapping);\n+\tfree(graph->old_mapping);\n+\tfree(graph);\n+}\n+\n static void graph_update_state(struct git_graph *graph, enum graph_state s)\n {\n \tgraph->prev_state = graph->state;\ndiff --git a/graph.h b/graph.h\nindex 8313e293c7..e88632a014 100644\n--- a/graph.h\n+++ b/graph.h\n@@ -139,6 +139,11 @@ void graph_set_column_colors(const char **colors, unsigned short colors_max);\n  */\n struct git_graph *graph_init(struct rev_info *opt);\n \n+/*\n+ * Free a struct git_graph.\n+ */\n+void graph_clear(struct git_graph *graph);\n+\n /*\n  * Update a git_graph with a new commit.\n  * This will cause the graph to begin outputting lines for the new commit\ndiff --git a/revision.c b/revision.c\nindex ad4286fbdd..816061f3d9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2426,6 +2426,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--graph\")) {\n \t\trevs->topo_order = 1;\n \t\trevs->rewrite_parents = 1;\n+\t\tgraph_clear(revs->graph);\n \t\trevs->graph = graph_init(revs);\n \t} else if (!strcmp(arg, \"--encode-email-headers\")) {\n \t\trevs->encode_email_headers = 1;\n-- \n2.32.0.2645.gc109162a1f\n\n"},{"id":"448168","messageId":"20220210165209.310930-2-alexhenrie24@gmail.com","threadId":"57401","inReplyTo":"20220210165209.310930-1-alexhenrie24@gmail.com","subject":"[PATCH v2 2/4] log: add a --no-graph option","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-02-10T16:52:07Z","receivedAt":"2022-02-10T16:53:14Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"It's useful to be able to countermand a previous --graph option, for\nexample if `git log --graph` is run via an alias.\n\nSigned-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\nv2: new patch\n---\n builtin/blame.c    |  1 +\n builtin/shortlog.c |  1 +\n revision.c         | 19 ++++++++++---\n revision.h         |  1 +\n t/t4202-log.sh     | 68 ++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 86 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 7fafeac408..ef831de5ac 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -934,6 +934,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tparse_revision_opt(&revs, &ctx, options, blame_opt_usage);\n \t}\n parse_done:\n+\trevision_opts_finish(&revs);\n \tno_whole_file_rename = !revs.diffopt.flags.follow_renames;\n \txdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;\n \trevs.diffopt.flags.follow_renames = 0;\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e7f7af5de3..228d782754 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -388,6 +388,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\tparse_revision_opt(&rev, &ctx, options, shortlog_usage);\n \t}\n parse_done:\n+\trevision_opts_finish(&rev);\n \targc = parse_options_end(&ctx);\n \n \tif (nongit && argc > 1) {\ndiff --git a/revision.c b/revision.c\nindex 816061f3d9..a39fd1c278 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2424,10 +2424,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->pretty_given = 1;\n \t\trevs->abbrev_commit = 1;\n \t} else if (!strcmp(arg, \"--graph\")) {\n-\t\trevs->topo_order = 1;\n-\t\trevs->rewrite_parents = 1;\n \t\tgraph_clear(revs->graph);\n \t\trevs->graph = graph_init(revs);\n+\t} else if (!strcmp(arg, \"--no-graph\")) {\n+\t\tgraph_clear(revs->graph);\n+\t\trevs->graph = NULL;\n \t} else if (!strcmp(arg, \"--encode-email-headers\")) {\n \t\trevs->encode_email_headers = 1;\n \t} else if (!strcmp(arg, \"--no-encode-email-headers\")) {\n@@ -2524,8 +2525,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\tunkv[(*unkc)++] = arg;\n \t\treturn opts;\n \t}\n-\tif (revs->graph && revs->track_linear)\n-\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--show-linear-break\", \"--graph\");\n \n \treturn 1;\n }\n@@ -2544,6 +2543,17 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \tctx->argc -= n;\n }\n \n+void revision_opts_finish(struct rev_info *revs)\n+{\n+\tif (revs->graph && revs->track_linear)\n+\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--show-linear-break\", \"--graph\");\n+\n+\tif (revs->graph) {\n+\t\trevs->topo_order = 1;\n+\t\trevs->rewrite_parents = 1;\n+\t}\n+}\n+\n static int for_each_bisect_ref(struct ref_store *refs, each_ref_fn fn,\n \t\t\t       void *cb_data, const char *term)\n {\n@@ -2786,6 +2796,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\tbreak;\n \t\t}\n \t}\n+\trevision_opts_finish(revs);\n \n \tif (prune_data.nr) {\n \t\t/*\ndiff --git a/revision.h b/revision.h\nindex 3f66147bfd..5a507db202 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -372,6 +372,7 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n #define REVARG_COMMITTISH 02\n int handle_revision_arg(const char *arg, struct rev_info *revs,\n \t\t\tint flags, unsigned revarg_opt);\n+void revision_opts_finish(struct rev_info *revs);\n \n /**\n  * Reset the flags used by the revision walking api. You can use this to do\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex dc884107de..f3f36a7e01 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1671,6 +1671,74 @@ test_expect_success 'log --graph with --name-only' '\n \ttest_cmp_graph --name-only tangle..reach\n '\n \n+test_expect_success '--no-graph countermands --graph' '\n+\tgit log >expect &&\n+\tgit log --graph --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--graph countermands --no-graph' '\n+\tgit log --graph >expect &&\n+\tgit log --no-graph --graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-graph does not unset --topo-order' '\n+\tgit log --topo-order >expect &&\n+\tgit log --topo-order --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-graph does not unset --parents' '\n+\tgit log --parents >expect &&\n+\tgit log --parents --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--reverse and --graph conflict' '\n+\ttest_must_fail git log --reverse --graph 2>stderr &&\n+\ttest_i18ngrep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success '--reverse --graph --no-graph works' '\n+\tgit log --reverse >expect &&\n+\tgit log --reverse --graph --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--show-linear-break and --graph conflict' '\n+\ttest_must_fail git log --show-linear-break --graph 2>stderr &&\n+\ttest_i18ngrep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success '--show-linear-break --graph --no-graph works' '\n+\tgit log --show-linear-break >expect &&\n+\tgit log --show-linear-break --graph --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-walk and --graph conflict' '\n+\ttest_must_fail git log --no-walk --graph 2>stderr &&\n+\ttest_i18ngrep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success '--no-walk --graph --no-graph works' '\n+\tgit log --no-walk >expect &&\n+\tgit log --no-walk --graph --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--walk-reflogs and --graph conflict' '\n+\ttest_must_fail git log --walk-reflogs --graph 2>stderr &&\n+\ttest_i18ngrep -E \"(cannot combine|cannot be used together)\" stderr\n+'\n+\n+test_expect_success '--walk-reflogs --graph --no-graph works' '\n+\tgit log --walk-reflogs >expect &&\n+\tgit log --walk-reflogs --graph --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'dotdot is a parent directory' '\n \tmkdir -p a/b &&\n \t( echo sixth && echo fifth ) >expect &&\n-- \n2.32.0.2645.gc109162a1f\n\n"},{"id":"448169","messageId":"20220210165209.310930-3-alexhenrie24@gmail.com","threadId":"57401","inReplyTo":"20220210165209.310930-1-alexhenrie24@gmail.com","subject":"[PATCH v2 3/4] log: add a log.graph config option","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-02-10T16:52:08Z","receivedAt":"2022-02-10T16:53:15Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"A coworker recently asked me how to turn on --graph by default in\n`git log`. I googled it and found that several people have asked that\nbefore on Stack Overflow, with no good solution:\nhttps://stackoverflow.com/questions/43555256/how-do-i-make-git-log-graph-the-default\n\nAdd a log.graph option to turn on graph mode in the absence of any\nincompatible options.\n\nSigned-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\nv2: ensure that --no-graph on the command line takes precedence over\nlog.graph=true in the config file\n---\n Documentation/config/log.txt |  5 +++++\n Documentation/git-log.txt    |  5 +++++\n builtin/log.c                |  6 ++++++\n revision.c                   | 10 +++++++++\n revision.h                   |  2 ++\n t/t4202-log.sh               | 42 ++++++++++++++++++++++++++++++++++++\n 6 files changed, 70 insertions(+)\n\ndiff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\nindex 456eb07800..3e356cfce6 100644\n--- a/Documentation/config/log.txt\n+++ b/Documentation/config/log.txt\n@@ -35,6 +35,11 @@ log.follow::\n \ti.e. it cannot be used to follow multiple files and does not work well\n \ton non-linear history.\n \n+log.graph::\n+\tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n+\tlinkgit:git-whatchanged[1] assume `--graph` unless an incompatible\n+\toption is also specified.\n+\n log.graphColors::\n \tA list of colors, separated by commas, that can be used to draw\n \thistory lines in `git log --graph`.\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 20e87cecf4..7e9e0f8afe 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -214,6 +214,11 @@ log.follow::\n \ti.e. it cannot be used to follow multiple files and does not work well\n \ton non-linear history.\n \n+log.graph::\n+\tIf `true`, `git log` and related commands will act as if the\n+\t`--graph` option was passed to them unless an incompatible option is\n+\talso specified.\n+\n log.showRoot::\n \tIf `false`, `git log` and related commands will not treat the\n \tinitial commit as a big creation event.  Any root commits in\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 4b493408cc..5eaefab046 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -48,6 +48,7 @@ static int default_show_root = 1;\n static int default_follow;\n static int default_show_signature;\n static int default_encode_email_headers = 1;\n+static int default_graph;\n static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config = 1;\n@@ -156,6 +157,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->show_signature = default_show_signature;\n \trev->encode_email_headers = default_encode_email_headers;\n \trev->diffopt.flags.allow_textconv = 1;\n+\trev->graph_default = default_graph;\n \n \tif (default_date_mode)\n \t\tparse_date_format(default_date_mode, &rev->date_mode);\n@@ -519,6 +521,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tdefault_show_signature = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"log.graph\")) {\n+\t\tdefault_graph = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\ndiff --git a/revision.c b/revision.c\nindex a39fd1c278..55e12cd401 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2426,9 +2426,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--graph\")) {\n \t\tgraph_clear(revs->graph);\n \t\trevs->graph = graph_init(revs);\n+\t\trevs->graph_default = 0;\n \t} else if (!strcmp(arg, \"--no-graph\")) {\n \t\tgraph_clear(revs->graph);\n \t\trevs->graph = NULL;\n+\t\trevs->graph_default = 0;\n \t} else if (!strcmp(arg, \"--encode-email-headers\")) {\n \t\trevs->encode_email_headers = 1;\n \t} else if (!strcmp(arg, \"--no-encode-email-headers\")) {\n@@ -2796,6 +2798,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\tbreak;\n \t\t}\n \t}\n+\tif (revs->graph_default &&\n+\t    !revs->graph &&\n+\t    /* check for incompatible options */\n+\t    !revs->track_linear &&\n+\t    !revs->reverse &&\n+\t    !revs->reflog_info &&\n+\t    !revs->no_walk)\n+\t\trevs->graph = graph_init(revs);\n \trevision_opts_finish(revs);\n \n \tif (prune_data.nr) {\ndiff --git a/revision.h b/revision.h\nindex 5a507db202..62a0ef4c53 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -249,6 +249,8 @@ struct rev_info {\n \n \t/* Display history graph */\n \tstruct git_graph *graph;\n+\t/* whether to initialize graph if it has not been initialized already */\n+\tint graph_default;\n \n \t/* special limits */\n \tint skip_count;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex f3f36a7e01..e9695510c3 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1695,6 +1695,20 @@ test_expect_success '--no-graph does not unset --parents' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log.graph=true behaves like --graph' '\n+\tgit log --graph >expect &&\n+\ttest_config log.graph true &&\n+\tgit log >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-graph countermands log.graph=true' '\n+\tgit log >expect &&\n+\ttest_config log.graph true &&\n+\tgit log --no-graph >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--reverse and --graph conflict' '\n \ttest_must_fail git log --reverse --graph 2>stderr &&\n \ttest_i18ngrep \"cannot be used together\" stderr\n@@ -1706,6 +1720,13 @@ test_expect_success '--reverse --graph --no-graph works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--reverse ignores log.graph' '\n+\tgit log --reverse >expect &&\n+\ttest_config log.graph true &&\n+\tgit log --reverse >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--show-linear-break and --graph conflict' '\n \ttest_must_fail git log --show-linear-break --graph 2>stderr &&\n \ttest_i18ngrep \"cannot be used together\" stderr\n@@ -1717,6 +1738,13 @@ test_expect_success '--show-linear-break --graph --no-graph works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--show-linear-break ignores log.graph' '\n+\tgit log --show-linear-break >expect &&\n+\ttest_config log.graph true &&\n+\tgit log --show-linear-break >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--no-walk and --graph conflict' '\n \ttest_must_fail git log --no-walk --graph 2>stderr &&\n \ttest_i18ngrep \"cannot be used together\" stderr\n@@ -1728,6 +1756,13 @@ test_expect_success '--no-walk --graph --no-graph works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--no-walk ignores log.graph' '\n+\tgit log --no-walk >expect &&\n+\ttest_config log.graph true &&\n+\tgit log --no-walk >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--walk-reflogs and --graph conflict' '\n \ttest_must_fail git log --walk-reflogs --graph 2>stderr &&\n \ttest_i18ngrep -E \"(cannot combine|cannot be used together)\" stderr\n@@ -1739,6 +1774,13 @@ test_expect_success '--walk-reflogs --graph --no-graph works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--walk-reflogs ignores log.graph' '\n+\tgit log --walk-reflogs >expect &&\n+\ttest_config log.graph true &&\n+\tgit log --walk-reflogs >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'dotdot is a parent directory' '\n \tmkdir -p a/b &&\n \t( echo sixth && echo fifth ) >expect &&\n-- \n2.32.0.2645.gc109162a1f\n\n"},{"id":"448170","messageId":"20220210165209.310930-4-alexhenrie24@gmail.com","threadId":"57401","inReplyTo":"20220210165209.310930-1-alexhenrie24@gmail.com","subject":"[PATCH v2 4/4] gitk: pass --no-graph to `git log`","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-02-10T16:52:09Z","receivedAt":"2022-02-10T16:53:21Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\nv2: use --no-graph instead of log.graph=false\n---\n gitk-git/gitk | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 23d9dd1fe0..24099ce0b8 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -411,8 +411,8 @@ proc start_rev_list {view} {\n     }\n \n     if {[catch {\n-        set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \\\n-                        --parents --boundary $args \"--\" $files] r]\n+        set fd [open [concat | git log --no-color --no-graph -z --pretty=raw \\\n+                        $show_notes --parents --boundary $args \"--\" $files] r]\n     } err]} {\n         error_popup \"[mc \"Error executing git log:\"] $err\"\n         return 0\n@@ -559,8 +559,9 @@ proc updatecommits {} {\n         set args $vorigargs($view)\n     }\n     if {[catch {\n-        set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \\\n-                        --parents --boundary $args \"--\" $vfilelimit($view)] r]\n+        set fd [open [concat | git log --no-color --no-graph -z --pretty=raw\n+                        $show_notes --parents --boundary $args \"--\"\n+                        $vfilelimit($view)] r]\n     } err]} {\n         error_popup \"[mc \"Error executing git log:\"] $err\"\n         return\n-- \n2.32.0.2645.gc109162a1f\n\n"}]}