| Message ID | patch-01.10-f3a4ed79c7d-20221230T071741Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sequencer API & users: fix widespread leaks | expand |
Hi Ævar On 30/12/2022 07:28, Ævar Arnfjörð Bjarmason wrote: > Use a "goto cleanup" pattern in do_interactive_rebase(). This > eliminates some duplicated free() code added in 0609b741a43 (rebase > -i: combine rebase--interactive.c with rebase.c, 2019-04-17), I read this as meaning that commit added some code to this function, but it fact it just copied the function unchanged from another file. > and sets > us up for a subsequent commit which'll make further use of the > "cleanup" label. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/rebase.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 1481c5b6a5b..7141fd5e0c1 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -256,7 +256,7 @@ static void split_exec_commands(const char *cmd, struct string_list *commands) > > static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) > { > - int ret; > + int ret = -1; > char *revisions = NULL, *shortrevisions = NULL; > struct strvec make_script_args = STRVEC_INIT; > struct todo_list todo_list = TODO_LIST_INIT; > @@ -265,16 +265,12 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) > > if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head->object.oid, > &revisions, &shortrevisions)) > - return -1; > + goto cleanup; This sort of change potentially problematic as we're free()ing things that were not previously free()d but revisions and shortrevisions are initialized to NULL before passing them to get_revision_ranges() so it is safe. Looks good Phillip > if (init_basic_state(&replay, > opts->head_name ? opts->head_name : "detached HEAD", > - opts->onto, &opts->orig_head->object.oid)) { > - free(revisions); > - free(shortrevisions); > - > - return -1; > - } > + opts->onto, &opts->orig_head->object.oid)) > + goto cleanup; > > if (!opts->upstream && opts->squash_onto) > write_file(path_squash_onto(), "%s\n", > @@ -304,6 +300,7 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) > opts->autosquash, opts->update_refs, &todo_list); > } > > +cleanup: > string_list_clear(&commands, 0); > free(revisions); > free(shortrevisions);
diff --git a/builtin/rebase.c b/builtin/rebase.c index 1481c5b6a5b..7141fd5e0c1 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -256,7 +256,7 @@ static void split_exec_commands(const char *cmd, struct string_list *commands) static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) { - int ret; + int ret = -1; char *revisions = NULL, *shortrevisions = NULL; struct strvec make_script_args = STRVEC_INIT; struct todo_list todo_list = TODO_LIST_INIT; @@ -265,16 +265,12 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head->object.oid, &revisions, &shortrevisions)) - return -1; + goto cleanup; if (init_basic_state(&replay, opts->head_name ? opts->head_name : "detached HEAD", - opts->onto, &opts->orig_head->object.oid)) { - free(revisions); - free(shortrevisions); - - return -1; - } + opts->onto, &opts->orig_head->object.oid)) + goto cleanup; if (!opts->upstream && opts->squash_onto) write_file(path_squash_onto(), "%s\n", @@ -304,6 +300,7 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) opts->autosquash, opts->update_refs, &todo_list); } +cleanup: string_list_clear(&commands, 0); free(revisions); free(shortrevisions);
Use a "goto cleanup" pattern in do_interactive_rebase(). This eliminates some duplicated free() code added in 0609b741a43 (rebase -i: combine rebase--interactive.c with rebase.c, 2019-04-17), and sets us up for a subsequent commit which'll make further use of the "cleanup" label. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/rebase.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)