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@xxxxxxxxx>
---
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);
