sequencer: release todo list on error paths
We're not releasing the `todo_list` in `sequencer_pick_revisions()` when hitting an error path. Restructure the function to have a common exit path such that we can easily clean up the list and thus plug this memory leak. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Aug 14, 2024 at 08:52 UTC
2f07d228c3a1c623526ddf38e609dc45c98fb22e
2 files changed
+48
-19
sequencer.c
+47
-19
@@ -5490,8 +5490,10 @@ int sequencer_pick_revisions(struct repository *r,
5490
int i, res;
5491
5492
assert(opts->revs);
5493
- if (read_and_refresh_cache(r, opts))
5494
- return -1;
5493
+ if (read_and_refresh_cache(r, opts)) {
5494
+ res = -1;
5495
+ goto out;
5496
+ }
5497
5498
for (i = 0; i < opts->revs->pending.nr; i++) {
5499
struct object_id oid;
@@ -5506,11 +5508,14 @@ int sequencer_pick_revisions(struct repository *r,
5508
enum object_type type = oid_object_info(r,
5509
&oid,
5510
NULL);
5509
- return error(_("%s: can't cherry-pick a %s"),
5510
- name, type_name(type));
5511
+ res = error(_("%s: can't cherry-pick a %s"),
5512
+ name, type_name(type));
5513
+ goto out;
5514
}
5512
- } else
5513
- return error(_("%s: bad revision"), name);
5515
+ } else {
5516
+ res = error(_("%s: bad revision"), name);
5517
+ goto out;
5518
+ }
5519
}
5520
5521
/*
@@ -5525,14 +5530,23 @@ int sequencer_pick_revisions(struct repository *r,
5530
opts->revs->no_walk &&
5531
!opts->revs->cmdline.rev->flags) {
5532
struct commit *cmit;
5528
- if (prepare_revision_walk(opts->revs))
5529
- return error(_("revision walk setup failed"));
5533
+
5534
+ if (prepare_revision_walk(opts->revs)) {
5535
+ res = error(_("revision walk setup failed"));
5536
+ goto out;
5537
+ }
5538
+
5539
cmit = get_revision(opts->revs);
5531
- if (!cmit)
5532
- return error(_("empty commit set passed"));
5540
+ if (!cmit) {
5541
+ res = error(_("empty commit set passed"));
5542
+ goto out;
5543
+ }
5544
+
5545
if (get_revision(opts->revs))
5546
BUG("unexpected extra commit from walk");
5535
- return single_pick(r, cmit, opts);
5547
+
5548
+ res = single_pick(r, cmit, opts);
5549
+ goto out;
5550
}
5551
5552
/*
@@ -5542,16 +5556,30 @@ int sequencer_pick_revisions(struct repository *r,
5556
*/
5557
5558
if (walk_revs_populate_todo(&todo_list, opts) ||
5545
- create_seq_dir(r) < 0)
5546
- return -1;
5547
- if (repo_get_oid(r, "HEAD", &oid) && (opts->action == REPLAY_REVERT))
5548
- return error(_("can't revert as initial commit"));
5549
- if (save_head(oid_to_hex(&oid)))
5550
- return -1;
5551
- if (save_opts(opts))
5552
- return -1;
5559
+ create_seq_dir(r) < 0) {
5560
+ res = -1;
5561
+ goto out;
5562
+ }
5563
+
5564
+ if (repo_get_oid(r, "HEAD", &oid) && (opts->action == REPLAY_REVERT)) {
5565
+ res = error(_("can't revert as initial commit"));
5566
+ goto out;
5567
+ }
5568
+
5569
+ if (save_head(oid_to_hex(&oid))) {
5570
+ res = -1;
5571
+ goto out;
5572
+ }
5573
+
5574
+ if (save_opts(opts)) {
5575
+ res = -1;
5576
+ goto out;
5577
+ }
5578
+
5579
update_abort_safety_file();
5580
res = pick_commits(r, &todo_list, opts);
5581
+
5582
+out:
5583
todo_list_release(&todo_list);
5584
return res;
5585
}
t/t3510-cherry-pick-sequence.sh
+1
@@ -12,6 +12,7 @@ test_description='Test cherry-pick continuation features
12
13
'
14
15
+TEST_PASSES_SANITIZE_LEAK=true
16
. ./test-lib.sh
17
18
# Repeat first match 10 times