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>

diff --git a/sequencer.c b/sequencer.c
index cade9b0ca873f430ddca3fe683fc423600525f78..ea559c31f1f66f1e7846f39cf18b23d94e2cbf45 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -5490,8 +5490,10 @@ int sequencer_pick_revisions(struct repository *r,
        int i, res;
 
        assert(opts->revs);
-       if (read_and_refresh_cache(r, opts))
-               return -1;
+       if (read_and_refresh_cache(r, opts)) {
+               res = -1;
+               goto out;
+       }
 
        for (i = 0; i < opts->revs->pending.nr; i++) {
                struct object_id oid;
@@ -5506,11 +5508,14 @@ int sequencer_pick_revisions(struct repository *r,
                                enum object_type type = oid_object_info(r,
                                                                        &oid,
                                                                        NULL);
-                               return error(_("%s: can't cherry-pick a %s"),
-                                       name, type_name(type));
+                               res = error(_("%s: can't cherry-pick a %s"),
+                                           name, type_name(type));
+                               goto out;
                        }
-               } else
-                       return error(_("%s: bad revision"), name);
+               } else {
+                       res = error(_("%s: bad revision"), name);
+                       goto out;
+               }
        }
 
        /*
@@ -5525,14 +5530,23 @@ int sequencer_pick_revisions(struct repository *r,
            opts->revs->no_walk &&
            !opts->revs->cmdline.rev->flags) {
                struct commit *cmit;
-               if (prepare_revision_walk(opts->revs))
-                       return error(_("revision walk setup failed"));
+
+               if (prepare_revision_walk(opts->revs)) {
+                       res = error(_("revision walk setup failed"));
+                       goto out;
+               }
+
                cmit = get_revision(opts->revs);
-               if (!cmit)
-                       return error(_("empty commit set passed"));
+               if (!cmit) {
+                       res = error(_("empty commit set passed"));
+                       goto out;
+               }
+
                if (get_revision(opts->revs))
                        BUG("unexpected extra commit from walk");
-               return single_pick(r, cmit, opts);
+
+               res = single_pick(r, cmit, opts);
+               goto out;
        }
 
        /*
@@ -5542,16 +5556,30 @@ int sequencer_pick_revisions(struct repository *r,
         */
 
        if (walk_revs_populate_todo(&todo_list, opts) ||
-                       create_seq_dir(r) < 0)
-               return -1;
-       if (repo_get_oid(r, "HEAD", &oid) && (opts->action == REPLAY_REVERT))
-               return error(_("can't revert as initial commit"));
-       if (save_head(oid_to_hex(&oid)))
-               return -1;
-       if (save_opts(opts))
-               return -1;
+                       create_seq_dir(r) < 0) {
+               res = -1;
+               goto out;
+       }
+
+       if (repo_get_oid(r, "HEAD", &oid) && (opts->action == REPLAY_REVERT)) {
+               res = error(_("can't revert as initial commit"));
+               goto out;
+       }
+
+       if (save_head(oid_to_hex(&oid))) {
+               res = -1;
+               goto out;
+       }
+
+       if (save_opts(opts)) {
+               res = -1;
+               goto out;
+       }
+
        update_abort_safety_file();
        res = pick_commits(r, &todo_list, opts);
+
+out:
        todo_list_release(&todo_list);
        return res;
 }

diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7eb52b12edc55702f48812725bfddc282297f9a5..93c725bac3cb3edd9fdaab7f0ecf9985fd7f848e 100755 (executable)
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -12,6 +12,7 @@ test_description='Test cherry-pick continuation features
 
 '
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # Repeat first match 10 times