Skip to content

Commit 365529e

Browse files
committed
Merge branch 'ps/leakfixes-part-7'
More leak-fixes. * ps/leakfixes-part-7: (23 commits) diffcore-break: fix leaking filespecs when merging broken pairs revision: fix leaking parents when simplifying commits builtin/maintenance: fix leak in `get_schedule_cmd()` builtin/maintenance: fix leaking config string promisor-remote: fix leaking partial clone filter grep: fix leaking grep pattern submodule: fix leaking submodule ODB paths trace2: destroy context stored in thread-local storage builtin/difftool: plug several trivial memory leaks builtin/repack: fix leaking configuration diffcore-order: fix leaking buffer when parsing orderfiles parse-options: free previous value of `OPTION_FILENAME` diff: fix leaking orderfile option builtin/pull: fix leaking "ff" option dir: fix off by one errors for ignored and untracked entries builtin/submodule--helper: fix leaking remote ref on errors t/helper: fix leaking subrepo in nested submodule config helper builtin/submodule--helper: fix leaking error buffer builtin/submodule--helper: clear child process when not running it submodule: fix leaking update strategy ...
2 parents 9293a93 + 12dfc24 commit 365529e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+279
-124
lines changed

builtin/difftool.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
662662
if (fp)
663663
fclose(fp);
664664

665+
hashmap_clear_and_free(&working_tree_dups, struct working_tree_entry, entry);
666+
hashmap_clear_and_free(&wt_modified, struct path_entry, entry);
667+
hashmap_clear_and_free(&tmp_modified, struct path_entry, entry);
668+
hashmap_clear_and_free(&submodules, struct pair_entry, entry);
669+
hashmap_clear_and_free(&symlinks2, struct pair_entry, entry);
670+
release_index(&wtindex);
665671
free(lbase_dir);
666672
free(rbase_dir);
667673
strbuf_release(&info);

builtin/gc.c

Lines changed: 82 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,9 +1478,9 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
14781478

14791479
static void initialize_maintenance_strategy(void)
14801480
{
1481-
char *config_str;
1481+
const char *config_str;
14821482

1483-
if (git_config_get_string("maintenance.strategy", &config_str))
1483+
if (git_config_get_string_tmp("maintenance.strategy", &config_str))
14841484
return;
14851485

14861486
if (!strcasecmp(config_str, "incremental")) {
@@ -1818,32 +1818,33 @@ static const char *get_extra_launchctl_strings(void) {
18181818
* * If $GIT_TEST_MAINT_SCHEDULER is set, return true.
18191819
* In this case, the *cmd value is read as input.
18201820
*
1821-
* * if the input value *cmd is the key of one of the comma-separated list
1822-
* item, then *is_available is set to true and *cmd is modified and becomes
1821+
* * if the input value cmd is the key of one of the comma-separated list
1822+
* item, then *is_available is set to true and *out is set to
18231823
* the mock command.
18241824
*
18251825
* * if the input value *cmd isn’t the key of any of the comma-separated list
1826-
* item, then *is_available is set to false.
1826+
* item, then *is_available is set to false and *out is set to the original
1827+
* command.
18271828
*
18281829
* Ex.:
18291830
* GIT_TEST_MAINT_SCHEDULER not set
18301831
* +-------+-------------------------------------------------+
18311832
* | Input | Output |
1832-
* | *cmd | return code | *cmd | *is_available |
1833+
* | *cmd | return code | *out | *is_available |
18331834
* +-------+-------------+-------------------+---------------+
1834-
* | "foo" | false | "foo" (unchanged) | (unchanged) |
1835+
* | "foo" | false | NULL | (unchanged) |
18351836
* +-------+-------------+-------------------+---------------+
18361837
*
18371838
* GIT_TEST_MAINT_SCHEDULER set to “foo:./mock_foo.sh,bar:./mock_bar.sh”
18381839
* +-------+-------------------------------------------------+
18391840
* | Input | Output |
1840-
* | *cmd | return code | *cmd | *is_available |
1841+
* | *cmd | return code | *out | *is_available |
18411842
* +-------+-------------+-------------------+---------------+
18421843
* | "foo" | true | "./mock.foo.sh" | true |
1843-
* | "qux" | true | "qux" (unchanged) | false |
1844+
* | "qux" | true | "qux" (allocated) | false |
18441845
* +-------+-------------+-------------------+---------------+
18451846
*/
1846-
static int get_schedule_cmd(const char **cmd, int *is_available)
1847+
static int get_schedule_cmd(const char *cmd, int *is_available, char **out)
18471848
{
18481849
char *testing = xstrdup_or_null(getenv("GIT_TEST_MAINT_SCHEDULER"));
18491850
struct string_list_item *item;
@@ -1862,16 +1863,22 @@ static int get_schedule_cmd(const char **cmd, int *is_available)
18621863
if (string_list_split_in_place(&pair, item->string, ":", 2) != 2)
18631864
continue;
18641865

1865-
if (!strcmp(*cmd, pair.items[0].string)) {
1866-
*cmd = pair.items[1].string;
1866+
if (!strcmp(cmd, pair.items[0].string)) {
1867+
if (out)
1868+
*out = xstrdup(pair.items[1].string);
18671869
if (is_available)
18681870
*is_available = 1;
1869-
string_list_clear(&list, 0);
1870-
UNLEAK(testing);
1871-
return 1;
1871+
string_list_clear(&pair, 0);
1872+
goto out;
18721873
}
1874+
1875+
string_list_clear(&pair, 0);
18731876
}
18741877

1878+
if (out)
1879+
*out = xstrdup(cmd);
1880+
1881+
out:
18751882
string_list_clear(&list, 0);
18761883
free(testing);
18771884
return 1;
@@ -1888,9 +1895,8 @@ static int get_random_minute(void)
18881895

18891896
static int is_launchctl_available(void)
18901897
{
1891-
const char *cmd = "launchctl";
18921898
int is_available;
1893-
if (get_schedule_cmd(&cmd, &is_available))
1899+
if (get_schedule_cmd("launchctl", &is_available, NULL))
18941900
return is_available;
18951901

18961902
#ifdef __APPLE__
@@ -1928,12 +1934,12 @@ static char *launchctl_get_uid(void)
19281934

19291935
static int launchctl_boot_plist(int enable, const char *filename)
19301936
{
1931-
const char *cmd = "launchctl";
1937+
char *cmd;
19321938
int result;
19331939
struct child_process child = CHILD_PROCESS_INIT;
19341940
char *uid = launchctl_get_uid();
19351941

1936-
get_schedule_cmd(&cmd, NULL);
1942+
get_schedule_cmd("launchctl", NULL, &cmd);
19371943
strvec_split(&child.args, cmd);
19381944
strvec_pushl(&child.args, enable ? "bootstrap" : "bootout", uid,
19391945
filename, NULL);
@@ -1946,6 +1952,7 @@ static int launchctl_boot_plist(int enable, const char *filename)
19461952

19471953
result = finish_command(&child);
19481954

1955+
free(cmd);
19491956
free(uid);
19501957
return result;
19511958
}
@@ -1997,10 +2004,10 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
19972004
static unsigned long lock_file_timeout_ms = ULONG_MAX;
19982005
struct strbuf plist = STRBUF_INIT, plist2 = STRBUF_INIT;
19992006
struct stat st;
2000-
const char *cmd = "launchctl";
2007+
char *cmd;
20012008
int minute = get_random_minute();
20022009

2003-
get_schedule_cmd(&cmd, NULL);
2010+
get_schedule_cmd("launchctl", NULL, &cmd);
20042011
preamble = "<?xml version=\"1.0\"?>\n"
20052012
"<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
20062013
"<plist version=\"1.0\">"
@@ -2092,6 +2099,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
20922099

20932100
free(filename);
20942101
free(name);
2102+
free(cmd);
20952103
strbuf_release(&plist);
20962104
strbuf_release(&plist2);
20972105
return 0;
@@ -2116,9 +2124,8 @@ static int launchctl_update_schedule(int run_maintenance, int fd UNUSED)
21162124

21172125
static int is_schtasks_available(void)
21182126
{
2119-
const char *cmd = "schtasks";
21202127
int is_available;
2121-
if (get_schedule_cmd(&cmd, &is_available))
2128+
if (get_schedule_cmd("schtasks", &is_available, NULL))
21222129
return is_available;
21232130

21242131
#ifdef GIT_WINDOWS_NATIVE
@@ -2137,15 +2144,16 @@ static char *schtasks_task_name(const char *frequency)
21372144

21382145
static int schtasks_remove_task(enum schedule_priority schedule)
21392146
{
2140-
const char *cmd = "schtasks";
2147+
char *cmd;
21412148
struct child_process child = CHILD_PROCESS_INIT;
21422149
const char *frequency = get_frequency(schedule);
21432150
char *name = schtasks_task_name(frequency);
21442151

2145-
get_schedule_cmd(&cmd, NULL);
2152+
get_schedule_cmd("schtasks", NULL, &cmd);
21462153
strvec_split(&child.args, cmd);
21472154
strvec_pushl(&child.args, "/delete", "/tn", name, "/f", NULL);
21482155
free(name);
2156+
free(cmd);
21492157

21502158
return run_command(&child);
21512159
}
@@ -2159,7 +2167,7 @@ static int schtasks_remove_tasks(void)
21592167

21602168
static int schtasks_schedule_task(const char *exec_path, enum schedule_priority schedule)
21612169
{
2162-
const char *cmd = "schtasks";
2170+
char *cmd;
21632171
int result;
21642172
struct child_process child = CHILD_PROCESS_INIT;
21652173
const char *xml;
@@ -2169,7 +2177,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
21692177
struct strbuf tfilename = STRBUF_INIT;
21702178
int minute = get_random_minute();
21712179

2172-
get_schedule_cmd(&cmd, NULL);
2180+
get_schedule_cmd("schtasks", NULL, &cmd);
21732181

21742182
strbuf_addf(&tfilename, "%s/schedule_%s_XXXXXX",
21752183
repo_get_common_dir(the_repository), frequency);
@@ -2276,6 +2284,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
22762284

22772285
delete_tempfile(&tfile);
22782286
free(name);
2287+
free(cmd);
22792288
return result;
22802289
}
22812290

@@ -2317,29 +2326,36 @@ static int check_crontab_process(const char *cmd)
23172326

23182327
static int is_crontab_available(void)
23192328
{
2320-
const char *cmd = "crontab";
2329+
char *cmd;
23212330
int is_available;
2331+
int ret;
23222332

2323-
if (get_schedule_cmd(&cmd, &is_available))
2324-
return is_available;
2333+
if (get_schedule_cmd("crontab", &is_available, &cmd)) {
2334+
ret = is_available;
2335+
goto out;
2336+
}
23252337

23262338
#ifdef __APPLE__
23272339
/*
23282340
* macOS has cron, but it requires special permissions and will
23292341
* create a UI alert when attempting to run this command.
23302342
*/
2331-
return 0;
2343+
ret = 0;
23322344
#else
2333-
return check_crontab_process(cmd);
2345+
ret = check_crontab_process(cmd);
23342346
#endif
2347+
2348+
out:
2349+
free(cmd);
2350+
return ret;
23352351
}
23362352

23372353
#define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE"
23382354
#define END_LINE "# END GIT MAINTENANCE SCHEDULE"
23392355

23402356
static int crontab_update_schedule(int run_maintenance, int fd)
23412357
{
2342-
const char *cmd = "crontab";
2358+
char *cmd;
23432359
int result = 0;
23442360
int in_old_region = 0;
23452361
struct child_process crontab_list = CHILD_PROCESS_INIT;
@@ -2349,15 +2365,17 @@ static int crontab_update_schedule(int run_maintenance, int fd)
23492365
struct tempfile *tmpedit = NULL;
23502366
int minute = get_random_minute();
23512367

2352-
get_schedule_cmd(&cmd, NULL);
2368+
get_schedule_cmd("crontab", NULL, &cmd);
23532369
strvec_split(&crontab_list.args, cmd);
23542370
strvec_push(&crontab_list.args, "-l");
23552371
crontab_list.in = -1;
23562372
crontab_list.out = dup(fd);
23572373
crontab_list.git_cmd = 0;
23582374

2359-
if (start_command(&crontab_list))
2360-
return error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
2375+
if (start_command(&crontab_list)) {
2376+
result = error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
2377+
goto out;
2378+
}
23612379

23622380
/* Ignore exit code, as an empty crontab will return error. */
23632381
finish_command(&crontab_list);
@@ -2427,8 +2445,10 @@ static int crontab_update_schedule(int run_maintenance, int fd)
24272445
result = error(_("'crontab' died"));
24282446
else
24292447
fclose(cron_list);
2448+
24302449
out:
24312450
delete_tempfile(&tmpedit);
2451+
free(cmd);
24322452
return result;
24332453
}
24342454

@@ -2451,10 +2471,9 @@ static int real_is_systemd_timer_available(void)
24512471

24522472
static int is_systemd_timer_available(void)
24532473
{
2454-
const char *cmd = "systemctl";
24552474
int is_available;
24562475

2457-
if (get_schedule_cmd(&cmd, &is_available))
2476+
if (get_schedule_cmd("systemctl", &is_available, NULL))
24582477
return is_available;
24592478

24602479
return real_is_systemd_timer_available();
@@ -2635,9 +2654,10 @@ static int systemd_timer_enable_unit(int enable,
26352654
enum schedule_priority schedule,
26362655
int minute)
26372656
{
2638-
const char *cmd = "systemctl";
2657+
char *cmd = NULL;
26392658
struct child_process child = CHILD_PROCESS_INIT;
26402659
const char *frequency = get_frequency(schedule);
2660+
int ret;
26412661

26422662
/*
26432663
* Disabling the systemd unit while it is already disabled makes
@@ -2648,30 +2668,43 @@ static int systemd_timer_enable_unit(int enable,
26482668
* On the other hand, enabling a systemd unit which is already enabled
26492669
* produces no error.
26502670
*/
2651-
if (!enable)
2671+
if (!enable) {
26522672
child.no_stderr = 1;
2653-
else if (systemd_timer_write_timer_file(schedule, minute))
2654-
return -1;
2673+
} else if (systemd_timer_write_timer_file(schedule, minute)) {
2674+
ret = -1;
2675+
goto out;
2676+
}
26552677

2656-
get_schedule_cmd(&cmd, NULL);
2678+
get_schedule_cmd("systemctl", NULL, &cmd);
26572679
strvec_split(&child.args, cmd);
26582680
strvec_pushl(&child.args, "--user", enable ? "enable" : "disable",
26592681
"--now", NULL);
26602682
strvec_pushf(&child.args, SYSTEMD_UNIT_FORMAT, frequency, "timer");
26612683

2662-
if (start_command(&child))
2663-
return error(_("failed to start systemctl"));
2664-
if (finish_command(&child))
2684+
if (start_command(&child)) {
2685+
ret = error(_("failed to start systemctl"));
2686+
goto out;
2687+
}
2688+
2689+
if (finish_command(&child)) {
26652690
/*
26662691
* Disabling an already disabled systemd unit makes
26672692
* systemctl fail.
26682693
* Let's ignore this failure.
26692694
*
26702695
* Enabling an enabled systemd unit doesn't fail.
26712696
*/
2672-
if (enable)
2673-
return error(_("failed to run systemctl"));
2674-
return 0;
2697+
if (enable) {
2698+
ret = error(_("failed to run systemctl"));
2699+
goto out;
2700+
}
2701+
}
2702+
2703+
ret = 0;
2704+
2705+
out:
2706+
free(cmd);
2707+
return ret;
26752708
}
26762709

26772710
/*

0 commit comments

Comments
 (0)