builtin/log: stop using globals for log config

We're using global variables to store the log configuration. Many of these can be set both via the command line and via the config, and depending on how they are being set, they may contain allocated strings. This leads to hard-to-track memory ownership and memory leaks. Refactor the code to instead use a `struct log_config` that is being allocated on the stack. This allows us to more clearly scope the variables, track memory ownership and ultimately release the memory. This also prepares us for a change to `git_config_string()`, which will be adapted to have a `char **` out parameter instead of `const char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 27, 2024 at 13:46 UTC 106a54aecb21fb17956290aee4e9204aa29174e7
1 file changed +156 -103
builtin/log.c
+156 -103
@@ -48,22 +48,8 @@
48 #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
49 #define FORMAT_PATCH_NAME_MAX_DEFAULT 64
50
51 -/* Set a default date-time format for git log ("log.date" config variable) */
52 -static const char *default_date_mode = NULL;
53 -
54 -static int default_abbrev_commit;
55 -static int default_show_root = 1;
56 -static int default_follow;
57 -static int default_show_signature;
58 -static int default_encode_email_headers = 1;
59 -static int decoration_style;
60 -static int decoration_given;
61 -static int use_mailmap_config = 1;
51 static unsigned int force_in_body_from;
52 static int stdout_mboxrd;
64 -static const char *fmt_patch_subject_prefix = "PATCH";
65 -static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
66 -static const char *fmt_pretty;
53 static int format_no_prefix;
54
55 static const char * const builtin_log_usage[] = {
@@ -111,6 +97,39 @@ static int parse_decoration_style(const char *value)
97 return -1;
98 }
99
100 +struct log_config {
101 + int default_abbrev_commit;
102 + int default_show_root;
103 + int default_follow;
104 + int default_show_signature;
105 + int default_encode_email_headers;
106 + int decoration_style;
107 + int decoration_given;
108 + int use_mailmap_config;
109 + char *fmt_patch_subject_prefix;
110 + int fmt_patch_name_max;
111 + char *fmt_pretty;
112 + char *default_date_mode;
113 +};
114 +
115 +static void log_config_init(struct log_config *cfg)
116 +{
117 + memset(cfg, 0, sizeof(*cfg));
118 + cfg->default_show_root = 1;
119 + cfg->default_encode_email_headers = 1;
120 + cfg->use_mailmap_config = 1;
121 + cfg->fmt_patch_subject_prefix = xstrdup("PATCH");
122 + cfg->fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
123 + cfg->decoration_style = auto_decoration_style();
124 +}
125 +
126 +static void log_config_release(struct log_config *cfg)
127 +{
128 + free(cfg->default_date_mode);
129 + free(cfg->fmt_pretty);
130 + free(cfg->fmt_patch_subject_prefix);
131 +}
132 +
133 static int use_default_decoration_filter = 1;
134 static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
135 static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
@@ -127,20 +146,22 @@ static int clear_decorations_callback(const struct option *opt UNUSED,
146 return 0;
147 }
148
130 -static int decorate_callback(const struct option *opt UNUSED, const char *arg,
149 +static int decorate_callback(const struct option *opt, const char *arg,
150 int unset)
151 {
152 + struct log_config *cfg = opt->value;
153 +
154 if (unset)
134 - decoration_style = 0;
155 + cfg->decoration_style = 0;
156 else if (arg)
136 - decoration_style = parse_decoration_style(arg);
157 + cfg->decoration_style = parse_decoration_style(arg);
158 else
138 - decoration_style = DECORATE_SHORT_REFS;
159 + cfg->decoration_style = DECORATE_SHORT_REFS;
160
140 - if (decoration_style < 0)
161 + if (cfg->decoration_style < 0)
162 die(_("invalid --decorate option: %s"), arg);
163
143 - decoration_given = 1;
164 + cfg->decoration_given = 1;
165
166 return 0;
167 }
@@ -160,32 +181,26 @@ static int log_line_range_callback(const struct option *option, const char *arg,
181 return 0;
182 }
183
163 -static void init_log_defaults(void)
184 +static void cmd_log_init_defaults(struct rev_info *rev,
185 + struct log_config *cfg)
186 {
165 - init_diff_ui_defaults();
166 -
167 - decoration_style = auto_decoration_style();
168 -}
169 -
170 -static void cmd_log_init_defaults(struct rev_info *rev)
171 -{
172 - if (fmt_pretty)
173 - get_commit_format(fmt_pretty, rev);
174 - if (default_follow)
187 + if (cfg->fmt_pretty)
188 + get_commit_format(cfg->fmt_pretty, rev);
189 + if (cfg->default_follow)
190 rev->diffopt.flags.default_follow_renames = 1;
191 rev->verbose_header = 1;
192 init_diffstat_widths(&rev->diffopt);
193 rev->diffopt.flags.recursive = 1;
194 rev->diffopt.flags.allow_textconv = 1;
180 - rev->abbrev_commit = default_abbrev_commit;
181 - rev->show_root_diff = default_show_root;
182 - rev->subject_prefix = fmt_patch_subject_prefix;
183 - rev->patch_name_max = fmt_patch_name_max;
184 - rev->show_signature = default_show_signature;
185 - rev->encode_email_headers = default_encode_email_headers;
195 + rev->abbrev_commit = cfg->default_abbrev_commit;
196 + rev->show_root_diff = cfg->default_show_root;
197 + rev->subject_prefix = cfg->fmt_patch_subject_prefix;
198 + rev->patch_name_max = cfg->fmt_patch_name_max;
199 + rev->show_signature = cfg->default_show_signature;
200 + rev->encode_email_headers = cfg->default_encode_email_headers;
201
187 - if (default_date_mode)
188 - parse_date_format(default_date_mode, &rev->date_mode);
202 + if (cfg->default_date_mode)
203 + parse_date_format(cfg->default_date_mode, &rev->date_mode);
204 }
205
206 static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
@@ -233,7 +248,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
248 }
249
250 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
236 - struct rev_info *rev, struct setup_revision_opt *opt)
251 + struct rev_info *rev, struct setup_revision_opt *opt,
252 + struct log_config *cfg)
253 {
254 struct userformat_want w;
255 int quiet = 0, source = 0, mailmap;
@@ -258,7 +274,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
274 N_("pattern"), N_("only decorate refs that match <pattern>")),
275 OPT_STRING_LIST(0, "decorate-refs-exclude", &decorate_refs_exclude,
276 N_("pattern"), N_("do not decorate refs that match <pattern>")),
261 - OPT_CALLBACK_F(0, "decorate", NULL, NULL, N_("decorate options"),
277 + OPT_CALLBACK_F(0, "decorate", cfg, NULL, N_("decorate options"),
278 PARSE_OPT_OPTARG, decorate_callback),
279 OPT_CALLBACK('L', NULL, &line_cb, "range:file",
280 N_("trace the evolution of line range <start>,<end> or function :<funcname> in <file>"),
@@ -269,7 +285,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
285 line_cb.rev = rev;
286 line_cb.prefix = prefix;
287
272 - mailmap = use_mailmap_config;
288 + mailmap = cfg->use_mailmap_config;
289 argc = parse_options(argc, argv, prefix,
290 builtin_log_options, builtin_log_usage,
291 PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT |
@@ -314,8 +330,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
330 * "log --pretty=raw" is special; ignore UI oriented
331 * configuration variables such as decoration.
332 */
317 - if (!decoration_given)
318 - decoration_style = 0;
333 + if (!cfg->decoration_given)
334 + cfg->decoration_style = 0;
335 if (!rev->abbrev_commit_given)
336 rev->abbrev_commit = 0;
337 }
@@ -326,24 +342,24 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
342 * Disable decoration loading if the format will not
343 * show them anyway.
344 */
329 - decoration_style = 0;
330 - } else if (!decoration_style) {
345 + cfg->decoration_style = 0;
346 + } else if (!cfg->decoration_style) {
347 /*
348 * If we are going to show them, make sure we do load
349 * them here, but taking care not to override a
350 * specific style set by config or --decorate.
351 */
336 - decoration_style = DECORATE_SHORT_REFS;
352 + cfg->decoration_style = DECORATE_SHORT_REFS;
353 }
354 }
355
340 - if (decoration_style || rev->simplify_by_decoration) {
356 + if (cfg->decoration_style || rev->simplify_by_decoration) {
357 set_default_decoration_filter(&decoration_filter);
358
343 - if (decoration_style)
359 + if (cfg->decoration_style)
360 rev->show_decorations = 1;
361
346 - load_ref_decorations(&decoration_filter, decoration_style);
362 + load_ref_decorations(&decoration_filter, cfg->decoration_style);
363 }
364
365 if (rev->line_level_traverse)
@@ -353,16 +369,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
369 }
370
371 static void cmd_log_init(int argc, const char **argv, const char *prefix,
356 - struct rev_info *rev, struct setup_revision_opt *opt)
372 + struct rev_info *rev, struct setup_revision_opt *opt,
373 + struct log_config *cfg)
374 {
358 - cmd_log_init_defaults(rev);
359 - cmd_log_init_finish(argc, argv, prefix, rev, opt);
360 -}
361 -
362 -static int cmd_log_deinit(int ret, struct rev_info *rev)
363 -{
364 - release_revisions(rev);
365 - return ret;
375 + cmd_log_init_defaults(rev, cfg);
376 + cmd_log_init_finish(argc, argv, prefix, rev, opt, cfg);
377 }
378
379 /*
@@ -566,30 +577,37 @@ static int cmd_log_walk(struct rev_info *rev)
577 static int git_log_config(const char *var, const char *value,
578 const struct config_context *ctx, void *cb)
579 {
580 + struct log_config *cfg = cb;
581 const char *slot_name;
582
571 - if (!strcmp(var, "format.pretty"))
572 - return git_config_string(&fmt_pretty, var, value);
573 - if (!strcmp(var, "format.subjectprefix"))
574 - return git_config_string(&fmt_patch_subject_prefix, var, value);
583 + if (!strcmp(var, "format.pretty")) {
584 + FREE_AND_NULL(cfg->fmt_pretty);
585 + return git_config_string((const char **) &cfg->fmt_pretty, var, value);
586 + }
587 + if (!strcmp(var, "format.subjectprefix")) {
588 + FREE_AND_NULL(cfg->fmt_patch_subject_prefix);
589 + return git_config_string((const char **) &cfg->fmt_patch_subject_prefix, var, value);
590 + }
591 if (!strcmp(var, "format.filenamemaxlength")) {
576 - fmt_patch_name_max = git_config_int(var, value, ctx->kvi);
592 + cfg->fmt_patch_name_max = git_config_int(var, value, ctx->kvi);
593 return 0;
594 }
595 if (!strcmp(var, "format.encodeemailheaders")) {
580 - default_encode_email_headers = git_config_bool(var, value);
596 + cfg->default_encode_email_headers = git_config_bool(var, value);
597 return 0;
598 }
599 if (!strcmp(var, "log.abbrevcommit")) {
584 - default_abbrev_commit = git_config_bool(var, value);
600 + cfg->default_abbrev_commit = git_config_bool(var, value);
601 return 0;
602 }
587 - if (!strcmp(var, "log.date"))
588 - return git_config_string(&default_date_mode, var, value);
603 + if (!strcmp(var, "log.date")) {
604 + FREE_AND_NULL(cfg->default_date_mode);
605 + return git_config_string((const char **) &cfg->default_date_mode, var, value);
606 + }
607 if (!strcmp(var, "log.decorate")) {
590 - decoration_style = parse_decoration_style(value);
591 - if (decoration_style < 0)
592 - decoration_style = 0; /* maybe warn? */
608 + cfg->decoration_style = parse_decoration_style(value);
609 + if (cfg->decoration_style < 0)
610 + cfg->decoration_style = 0; /* maybe warn? */
611 return 0;
612 }
613 if (!strcmp(var, "log.diffmerges")) {
@@ -598,21 +616,21 @@ static int git_log_config(const char *var, const char *value,
616 return diff_merges_config(value);
617 }
618 if (!strcmp(var, "log.showroot")) {
601 - default_show_root = git_config_bool(var, value);
619 + cfg->default_show_root = git_config_bool(var, value);
620 return 0;
621 }
622 if (!strcmp(var, "log.follow")) {
605 - default_follow = git_config_bool(var, value);
623 + cfg->default_follow = git_config_bool(var, value);
624 return 0;
625 }
626 if (skip_prefix(var, "color.decorate.", &slot_name))
627 return parse_decorate_color_config(var, slot_name, value);
628 if (!strcmp(var, "log.mailmap")) {
611 - use_mailmap_config = git_config_bool(var, value);
629 + cfg->use_mailmap_config = git_config_bool(var, value);
630 return 0;
631 }
632 if (!strcmp(var, "log.showsignature")) {
615 - default_show_signature = git_config_bool(var, value);
633 + cfg->default_show_signature = git_config_bool(var, value);
634 return 0;
635 }
636
@@ -621,11 +639,14 @@ static int git_log_config(const char *var, const char *value,
639
640 int cmd_whatchanged(int argc, const char **argv, const char *prefix)
641 {
642 + struct log_config cfg;
643 struct rev_info rev;
644 struct setup_revision_opt opt;
645 + int ret;
646
627 - init_log_defaults();
628 - git_config(git_log_config, NULL);
647 + log_config_init(&cfg);
648 + init_diff_ui_defaults();
649 + git_config(git_log_config, &cfg);
650
651 repo_init_revisions(the_repository, &rev, prefix);
652 git_config(grep_config, &rev.grep_filter);
@@ -635,10 +656,15 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
656 memset(&opt, 0, sizeof(opt));
657 opt.def = "HEAD";
658 opt.revarg_opt = REVARG_COMMITTISH;
638 - cmd_log_init(argc, argv, prefix, &rev, &opt);
659 + cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg);
660 if (!rev.diffopt.output_format)
661 rev.diffopt.output_format = DIFF_FORMAT_RAW;
641 - return cmd_log_deinit(cmd_log_walk(&rev), &rev);
662 +
663 + ret = cmd_log_walk(&rev);
664 +
665 + release_revisions(&rev);
666 + log_config_release(&cfg);
667 + return ret;
668 }
669
670 static void show_tagger(const char *buf, struct rev_info *rev)
@@ -733,14 +759,16 @@ static void show_setup_revisions_tweak(struct rev_info *rev)
759
760 int cmd_show(int argc, const char **argv, const char *prefix)
761 {
762 + struct log_config cfg;
763 struct rev_info rev;
764 unsigned int i;
765 struct setup_revision_opt opt;
766 struct pathspec match_all;
767 int ret = 0;
768
742 - init_log_defaults();
743 - git_config(git_log_config, NULL);
769 + log_config_init(&cfg);
770 + init_diff_ui_defaults();
771 + git_config(git_log_config, &cfg);
772
773 if (the_repository->gitdir) {
774 prepare_repo_settings(the_repository);
@@ -759,10 +787,14 @@ int cmd_show(int argc, const char **argv, const char *prefix)
787 memset(&opt, 0, sizeof(opt));
788 opt.def = "HEAD";
789 opt.tweak = show_setup_revisions_tweak;
762 - cmd_log_init(argc, argv, prefix, &rev, &opt);
790 + cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg);
791
764 - if (!rev.no_walk)
765 - return cmd_log_deinit(cmd_log_walk(&rev), &rev);
792 + if (!rev.no_walk) {
793 + ret = cmd_log_walk(&rev);
794 + release_revisions(&rev);
795 + log_config_release(&cfg);
796 + return ret;
797 + }
798
799 rev.diffopt.no_free = 1;
800 for (i = 0; i < rev.pending.nr && !ret; i++) {
@@ -832,8 +864,10 @@ int cmd_show(int argc, const char **argv, const char *prefix)
864
865 rev.diffopt.no_free = 0;
866 diff_free(&rev.diffopt);
867 + release_revisions(&rev);
868 + log_config_release(&cfg);
869
836 - return cmd_log_deinit(ret, &rev);
870 + return ret;
871 }
872
873 /*
@@ -841,11 +875,14 @@ int cmd_show(int argc, const char **argv, const char *prefix)
875 */
876 int cmd_log_reflog(int argc, const char **argv, const char *prefix)
877 {
878 + struct log_config cfg;
879 struct rev_info rev;
880 struct setup_revision_opt opt;
881 + int ret;
882
847 - init_log_defaults();
848 - git_config(git_log_config, NULL);
883 + log_config_init(&cfg);
884 + init_diff_ui_defaults();
885 + git_config(git_log_config, &cfg);
886
887 repo_init_revisions(the_repository, &rev, prefix);
888 init_reflog_walk(&rev.reflog_info);
@@ -854,14 +891,18 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
891 rev.verbose_header = 1;
892 memset(&opt, 0, sizeof(opt));
893 opt.def = "HEAD";
857 - cmd_log_init_defaults(&rev);
894 + cmd_log_init_defaults(&rev, &cfg);
895 rev.abbrev_commit = 1;
896 rev.commit_format = CMIT_FMT_ONELINE;
897 rev.use_terminator = 1;
898 rev.always_show_header = 1;
862 - cmd_log_init_finish(argc, argv, prefix, &rev, &opt);
899 + cmd_log_init_finish(argc, argv, prefix, &rev, &opt, &cfg);
900 +
901 + ret = cmd_log_walk(&rev);
902
864 - return cmd_log_deinit(cmd_log_walk(&rev), &rev);
903 + release_revisions(&rev);
904 + log_config_release(&cfg);
905 + return ret;
906 }
907
908 static void log_setup_revisions_tweak(struct rev_info *rev)
@@ -876,11 +917,14 @@ static void log_setup_revisions_tweak(struct rev_info *rev)
917
918 int cmd_log(int argc, const char **argv, const char *prefix)
919 {
920 + struct log_config cfg;
921 struct rev_info rev;
922 struct setup_revision_opt opt;
923 + int ret;
924
882 - init_log_defaults();
883 - git_config(git_log_config, NULL);
925 + log_config_init(&cfg);
926 + init_diff_ui_defaults();
927 + git_config(git_log_config, &cfg);
928
929 repo_init_revisions(the_repository, &rev, prefix);
930 git_config(grep_config, &rev.grep_filter);
@@ -890,8 +934,13 @@ int cmd_log(int argc, const char **argv, const char *prefix)
934 opt.def = "HEAD";
935 opt.revarg_opt = REVARG_COMMITTISH;
936 opt.tweak = log_setup_revisions_tweak;
893 - cmd_log_init(argc, argv, prefix, &rev, &opt);
894 - return cmd_log_deinit(cmd_log_walk(&rev), &rev);
937 + cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg);
938 +
939 + ret = cmd_log_walk(&rev);
940 +
941 + release_revisions(&rev);
942 + log_config_release(&cfg);
943 + return ret;
944 }
945
946 /* format-patch */
@@ -1884,6 +1933,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,
1933
1934 int cmd_format_patch(int argc, const char **argv, const char *prefix)
1935 {
1936 + struct log_config cfg;
1937 struct commit *commit;
1938 struct commit **list = NULL;
1939 struct rev_info rev;
@@ -1943,7 +1993,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
1993 N_("start numbering patches at <n> instead of 1")),
1994 OPT_STRING('v', "reroll-count", &reroll_count, N_("reroll-count"),
1995 N_("mark the series as Nth re-roll")),
1946 - OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
1996 + OPT_INTEGER(0, "filename-max-length", &cfg.fmt_patch_name_max,
1997 N_("max length of output filename")),
1998 OPT_CALLBACK_F(0, "rfc", &rfc, N_("rfc"),
1999 N_("add <rfc> (default 'RFC') before 'PATCH'"),
@@ -2017,16 +2067,17 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
2067 extra_to.strdup_strings = 1;
2068 extra_cc.strdup_strings = 1;
2069
2020 - init_log_defaults();
2070 + log_config_init(&cfg);
2071 + init_diff_ui_defaults();
2072 init_display_notes(&notes_opt);
2022 - git_config(git_format_config, NULL);
2073 + git_config(git_format_config, &cfg);
2074 repo_init_revisions(the_repository, &rev, prefix);
2075 git_config(grep_config, &rev.grep_filter);
2076
2077 rev.show_notes = show_notes;
2078 memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
2079 rev.commit_format = CMIT_FMT_EMAIL;
2029 - rev.encode_email_headers = default_encode_email_headers;
2080 + rev.encode_email_headers = cfg.default_encode_email_headers;
2081 rev.expand_tabs_in_log_default = 0;
2082 rev.verbose_header = 1;
2083 rev.diff = 1;
@@ -2037,7 +2088,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
2088 s_r_opt.def = "HEAD";
2089 s_r_opt.revarg_opt = REVARG_COMMITTISH;
2090
2040 - strbuf_addstr(&sprefix, fmt_patch_subject_prefix);
2091 + strbuf_addstr(&sprefix, cfg.fmt_patch_subject_prefix);
2092 if (format_no_prefix)
2093 diff_set_noprefix(&rev.diffopt);
2094
@@ -2059,8 +2110,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
2110 rev.force_in_body_from = force_in_body_from;
2111
2112 /* Make sure "0000-$sub.patch" gives non-negative length for $sub */
2062 - if (fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
2063 - fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
2113 + if (cfg.fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
2114 + cfg.fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
2115
2116 if (cover_from_description_arg)
2117 cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
@@ -2156,7 +2207,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
2207 rev.always_show_header = 1;
2208
2209 rev.zero_commit = zero_commit;
2159 - rev.patch_name_max = fmt_patch_name_max;
2210 + rev.patch_name_max = cfg.fmt_patch_name_max;
2211
2212 if (!rev.diffopt.flags.text && !no_binary_diff)
2213 rev.diffopt.flags.binary = 1;
@@ -2450,7 +2501,9 @@ done:
2501 if (rev.ref_message_ids)
2502 string_list_clear(rev.ref_message_ids, 0);
2503 free(rev.ref_message_ids);
2453 - return cmd_log_deinit(0, &rev);
2504 + release_revisions(&rev);
2505 + log_config_release(&cfg);
2506 + return 0;
2507 }
2508
2509 static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)