| Message ID | patch-v2-4.9-e83bdfab046-20230112T124201Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sequencer API & users: fix widespread leaks | expand |
Hi Ævar On 12/01/2023 12:45, Ævar Arnfjörð Bjarmason wrote: > In [1] and [2] I added the code being moved here to cmd_revert() and > cmd_cherry_pick(), I'm not sure how relevant this first clause is but the change itself looks good. > now that we've got a "replay_opts_release()" for > the "struct replay_opts" it should know how to free these "revs", > rather than having these users reach into the struct to free its > individual members. > > As explained in earlier change we should be using FREE_AND_NULL() in > replay_opts_release() rather than free(). This paragraph is out of date Best Wishes Phillip > 1. d1ec656d68f (cherry-pick: free "struct replay_opts" members, > 2022-11-08) > 2. fd74ac95ac3 (revert: free "struct replay_opts" members, 2022-07-01) > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/revert.c | 6 ------ > sequencer.c | 3 +++ > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/builtin/revert.c b/builtin/revert.c > index 1cab16bf3ed..77d2035616e 100644 > --- a/builtin/revert.c > +++ b/builtin/revert.c > @@ -248,9 +248,6 @@ int cmd_revert(int argc, const char **argv, const char *prefix) > res = run_sequencer(argc, argv, &opts); > if (res < 0) > die(_("revert failed")); > - if (opts.revs) > - release_revisions(opts.revs); > - free(opts.revs); > replay_opts_release(&opts); > return res; > } > @@ -263,9 +260,6 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix) > opts.action = REPLAY_PICK; > sequencer_init_config(&opts); > res = run_sequencer(argc, argv, &opts); > - if (opts.revs) > - release_revisions(opts.revs); > - free(opts.revs); > if (res < 0) > die(_("cherry-pick failed")); > replay_opts_release(&opts); > diff --git a/sequencer.c b/sequencer.c > index 5d8c68912a1..c729ce77260 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -362,6 +362,9 @@ void replay_opts_release(struct replay_opts *opts) > opts->xopts_nr = 0; > free(opts->xopts); > strbuf_release(&opts->current_fixups); > + if (opts->revs) > + release_revisions(opts->revs); > + free(opts->revs); > } > > int sequencer_remove_state(struct replay_opts *opts)
diff --git a/builtin/revert.c b/builtin/revert.c index 1cab16bf3ed..77d2035616e 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -248,9 +248,6 @@ int cmd_revert(int argc, const char **argv, const char *prefix) res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("revert failed")); - if (opts.revs) - release_revisions(opts.revs); - free(opts.revs); replay_opts_release(&opts); return res; } @@ -263,9 +260,6 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix) opts.action = REPLAY_PICK; sequencer_init_config(&opts); res = run_sequencer(argc, argv, &opts); - if (opts.revs) - release_revisions(opts.revs); - free(opts.revs); if (res < 0) die(_("cherry-pick failed")); replay_opts_release(&opts); diff --git a/sequencer.c b/sequencer.c index 5d8c68912a1..c729ce77260 100644 --- a/sequencer.c +++ b/sequencer.c @@ -362,6 +362,9 @@ void replay_opts_release(struct replay_opts *opts) opts->xopts_nr = 0; free(opts->xopts); strbuf_release(&opts->current_fixups); + if (opts->revs) + release_revisions(opts->revs); + free(opts->revs); } int sequencer_remove_state(struct replay_opts *opts)
In [1] and [2] I added the code being moved here to cmd_revert() and cmd_cherry_pick(), now that we've got a "replay_opts_release()" for the "struct replay_opts" it should know how to free these "revs", rather than having these users reach into the struct to free its individual members. As explained in earlier change we should be using FREE_AND_NULL() in replay_opts_release() rather than free(). 1. d1ec656d68f (cherry-pick: free "struct replay_opts" members, 2022-11-08) 2. fd74ac95ac3 (revert: free "struct replay_opts" members, 2022-07-01) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/revert.c | 6 ------ sequencer.c | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-)