builtin/gc: fix leaking config values
We're leaking config values in git-gc(1) when those values are tracked as strings. Introduce a new `gc_config_release()` function that releases this memory to plug those leaks and release old values before populating the config fields via `git_config_string()` et al. Note that there is one small gotcha here with the "--prune" option. Next to passing a string, this option also accepts the "--no-prune" option that overrides the default or configured value. We thus need to discern between the option not having been passed by the user and the negative variant of it. This is done by using a simple sentinel value that lets us discern these cases. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Aug 16, 2024 at 12:45 UTC
0ce44e2293352bc3636d93c156fbc2a1ebac0e45
2 files changed
+82
-27
builtin/gc.c
+81
-27
@@ -139,9 +139,9 @@ struct gc_config {
139
int gc_auto_threshold;
140
int gc_auto_pack_limit;
141
int detach_auto;
142
- const char *gc_log_expire;
143
- const char *prune_expire;
144
- const char *prune_worktrees_expire;
142
+ char *gc_log_expire;
143
+ char *prune_expire;
144
+ char *prune_worktrees_expire;
145
char *repack_filter;
146
char *repack_filter_to;
147
unsigned long big_pack_threshold;
@@ -157,15 +157,25 @@ struct gc_config {
157
.gc_auto_threshold = 6700, \
158
.gc_auto_pack_limit = 50, \
159
.detach_auto = 1, \
160
- .gc_log_expire = "1.day.ago", \
161
- .prune_expire = "2.weeks.ago", \
162
- .prune_worktrees_expire = "3.months.ago", \
160
+ .gc_log_expire = xstrdup("1.day.ago"), \
161
+ .prune_expire = xstrdup("2.weeks.ago"), \
162
+ .prune_worktrees_expire = xstrdup("3.months.ago"), \
163
.max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \
164
}
165
166
+static void gc_config_release(struct gc_config *cfg)
167
+{
168
+ free(cfg->gc_log_expire);
169
+ free(cfg->prune_expire);
170
+ free(cfg->prune_worktrees_expire);
171
+ free(cfg->repack_filter);
172
+ free(cfg->repack_filter_to);
173
+}
174
+
175
static void gc_config(struct gc_config *cfg)
176
{
177
const char *value;
178
+ char *owned = NULL;
179
180
if (!git_config_get_value("gc.packrefs", &value)) {
181
if (value && !strcmp(value, "notbare"))
@@ -185,15 +195,34 @@ static void gc_config(struct gc_config *cfg)
195
git_config_get_bool("gc.autodetach", &cfg->detach_auto);
196
git_config_get_bool("gc.cruftpacks", &cfg->cruft_packs);
197
git_config_get_ulong("gc.maxcruftsize", &cfg->max_cruft_size);
188
- git_config_get_expiry("gc.pruneexpire", (char **) &cfg->prune_expire);
189
- git_config_get_expiry("gc.worktreepruneexpire", (char **) &cfg->prune_worktrees_expire);
190
- git_config_get_expiry("gc.logexpiry", (char **) &cfg->gc_log_expire);
198
+
199
+ if (!git_config_get_expiry("gc.pruneexpire", &owned)) {
200
+ free(cfg->prune_expire);
201
+ cfg->prune_expire = owned;
202
+ }
203
+
204
+ if (!git_config_get_expiry("gc.worktreepruneexpire", &owned)) {
205
+ free(cfg->prune_worktrees_expire);
206
+ cfg->prune_worktrees_expire = owned;
207
+ }
208
+
209
+ if (!git_config_get_expiry("gc.logexpiry", &owned)) {
210
+ free(cfg->gc_log_expire);
211
+ cfg->gc_log_expire = owned;
212
+ }
213
214
git_config_get_ulong("gc.bigpackthreshold", &cfg->big_pack_threshold);
215
git_config_get_ulong("pack.deltacachesize", &cfg->max_delta_cache_size);
216
195
- git_config_get_string("gc.repackfilter", &cfg->repack_filter);
196
- git_config_get_string("gc.repackfilterto", &cfg->repack_filter_to);
217
+ if (!git_config_get_string("gc.repackfilter", &owned)) {
218
+ free(cfg->repack_filter);
219
+ cfg->repack_filter = owned;
220
+ }
221
+
222
+ if (!git_config_get_string("gc.repackfilterto", &owned)) {
223
+ free(cfg->repack_filter_to);
224
+ cfg->repack_filter_to = owned;
225
+ }
226
227
git_config(git_default_config, NULL);
228
}
@@ -644,12 +673,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
673
struct child_process rerere_cmd = CHILD_PROCESS_INIT;
674
struct maintenance_run_opts opts = {0};
675
struct gc_config cfg = GC_CONFIG_INIT;
676
+ const char *prune_expire_sentinel = "sentinel";
677
+ const char *prune_expire_arg = prune_expire_sentinel;
678
+ int ret;
679
680
struct option builtin_gc_options[] = {
681
OPT__QUIET(&quiet, N_("suppress progress reporting")),
650
- { OPTION_STRING, 0, "prune", &cfg.prune_expire, N_("date"),
682
+ { OPTION_STRING, 0, "prune", &prune_expire_arg, N_("date"),
683
N_("prune unreferenced objects"),
652
- PARSE_OPT_OPTARG, NULL, (intptr_t)cfg.prune_expire },
684
+ PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire_arg },
685
OPT_BOOL(0, "cruft", &cfg.cruft_packs, N_("pack unreferenced objects separately")),
686
OPT_MAGNITUDE(0, "max-cruft-size", &cfg.max_cruft_size,
687
N_("with --cruft, limit the size of new cruft packs")),
@@ -673,8 +705,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
705
strvec_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL);
706
strvec_pushl(&rerere, "rerere", "gc", NULL);
707
676
- /* default expiry time, overwritten in gc_config */
708
gc_config(&cfg);
709
+
710
if (parse_expiry_date(cfg.gc_log_expire, &gc_log_expire_time))
711
die(_("failed to parse gc.logExpiry value %s"), cfg.gc_log_expire);
712
@@ -686,6 +718,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
718
if (argc > 0)
719
usage_with_options(builtin_gc_usage, builtin_gc_options);
720
721
+ if (prune_expire_arg != prune_expire_sentinel) {
722
+ free(cfg.prune_expire);
723
+ cfg.prune_expire = xstrdup_or_null(prune_expire_arg);
724
+ }
725
if (cfg.prune_expire && parse_expiry_date(cfg.prune_expire, &dummy))
726
die(_("failed to parse prune expiry value %s"), cfg.prune_expire);
727
@@ -703,8 +739,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
739
/*
740
* Auto-gc should be least intrusive as possible.
741
*/
706
- if (!need_to_gc(&cfg))
707
- return 0;
742
+ if (!need_to_gc(&cfg)) {
743
+ ret = 0;
744
+ goto out;
745
+ }
746
+
747
if (!quiet) {
748
if (cfg.detach_auto)
749
fprintf(stderr, _("Auto packing the repository in background for optimum performance.\n"));
@@ -713,17 +752,22 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
752
fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
753
}
754
if (cfg.detach_auto) {
716
- int ret = report_last_gc_error();
717
-
718
- if (ret == 1)
755
+ ret = report_last_gc_error();
756
+ if (ret == 1) {
757
/* Last gc --auto failed. Skip this one. */
720
- return 0;
721
- else if (ret)
758
+ ret = 0;
759
+ goto out;
760
+
761
+ } else if (ret) {
762
/* an I/O error occurred, already reported */
723
- return ret;
763
+ goto out;
764
+ }
765
+
766
+ if (lock_repo_for_gc(force, &pid)) {
767
+ ret = 0;
768
+ goto out;
769
+ }
770
725
- if (lock_repo_for_gc(force, &pid))
726
- return 0;
771
gc_before_repack(&opts, &cfg); /* dies on failure */
772
delete_tempfile(&pidfile);
773
@@ -749,8 +793,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
793
794
name = lock_repo_for_gc(force, &pid);
795
if (name) {
752
- if (opts.auto_flag)
753
- return 0; /* be quiet on --auto */
796
+ if (opts.auto_flag) {
797
+ ret = 0;
798
+ goto out; /* be quiet on --auto */
799
+ }
800
+
801
die(_("gc is already running on machine '%s' pid %"PRIuMAX" (use --force if not)"),
802
name, (uintmax_t)pid);
803
}
@@ -826,6 +873,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
873
if (!daemonized)
874
unlink(git_path("gc.log"));
875
876
+out:
877
+ gc_config_release(&cfg);
878
return 0;
879
}
880
@@ -1511,6 +1560,8 @@ static int maintenance_run(int argc, const char **argv, const char *prefix)
1560
PARSE_OPT_NONEG, task_option_parse),
1561
OPT_END()
1562
};
1563
+ int ret;
1564
+
1565
memset(&opts, 0, sizeof(opts));
1566
1567
opts.quiet = !isatty(2);
@@ -1532,7 +1583,10 @@ static int maintenance_run(int argc, const char **argv, const char *prefix)
1583
if (argc != 0)
1584
usage_with_options(builtin_maintenance_run_usage,
1585
builtin_maintenance_run_options);
1535
- return maintenance_run_tasks(&opts, &cfg);
1586
+
1587
+ ret = maintenance_run_tasks(&opts, &cfg);
1588
+ gc_config_release(&cfg);
1589
+ return ret;
1590
}
1591
1592
static char *get_maintpath(void)
t/t5304-prune.sh
+1
@@ -7,6 +7,7 @@ test_description='prune'
7
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
8
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
9
10
+TEST_PASSES_SANITIZE_LEAK=true
11
. ./test-lib.sh
12
13
day=$((60*60*24))