@matheusbsbs / matheus-bagliano / commits / 2cb9294b99

submodule--helper: don't exit() on failure, return

Change code downstream of module_update() to short-circuit and return to the top-level on failure, rather than calling exit(). To do so we need to diligently check whether we "must_die_on_failure", which is a pattern started in c51f8f94e5b (submodule--helper: run update procedures from C, 2021-08-24), but which hadn't been completed to the point where we could avoid calling exit() here. This introduces no functional changes, but makes it easier to both call these routines as a library in the future, and to eventually avoid leaking memory. This and similar control flow in submodule--helper.c could be made simpler by properly "libifying" it, i.e. to have it consistently return -1 on failures, and to early return on any non-success. But let's leave that larger project for now, and (mostly) emulate what were doing with the "exit(128)" before this change. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 1, 2022 at 01:18 UTC 2cb9294b99f62bb7439c99ba22898508ba4ff621

1 file changed +25 -10

builtin/submodule--helper.c

+25 -10

@@ -2123,7 +2123,8 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,

2123 return run_command(&cp);

2124 }

2125

2126 -static int run_update_command(const struct update_data *ud, int subforce)

2126 +static int run_update_command(const struct update_data *ud, int subforce,

2127 + int *must_die_on_failure)

2128 {

2129 struct child_process cp = CHILD_PROCESS_INIT;

2130 char *oid = oid_to_hex(&ud->oid);

@@ -2187,7 +2188,7 @@ static int run_update_command(const struct update_data *ud, int subforce)

2188 }

2189

2190 if (ret == 128)

2190 - exit(ret);

2191 + *must_die_on_failure = 1;

2192 return ret;

2193 }

2194

@@ -2219,7 +2220,8 @@ static int run_update_command(const struct update_data *ud, int subforce)

2220 return 0;

2221 }

2222

2222 -static int run_update_procedure(const struct update_data *ud)

2223 +static int run_update_procedure(const struct update_data *ud,

2224 + int *must_die_on_failure)

2225 {

2226 int subforce = is_null_oid(&ud->suboid) || ud->force;

2227

@@ -2246,7 +2248,7 @@ static int run_update_procedure(const struct update_data *ud)

2248 ud->displaypath, oid_to_hex(&ud->oid));

2249 }

2250

2249 - return run_update_command(ud, subforce);

2251 + return run_update_command(ud, subforce, must_die_on_failure);

2252 }

2253

2254 static const char *remote_submodule_branch(const char *path)

@@ -2383,7 +2385,8 @@ static void update_data_to_args(const struct update_data *update_data,

2385 "--no-single-branch");

2386 }

2387

2386 -static int update_submodule(struct update_data *update_data)

2388 +static int update_submodule(struct update_data *update_data,

2389 + int *must_die_on_failure)

2390 {

2391 ensure_core_worktree(update_data->sm_path);

2392

@@ -2419,9 +2422,15 @@ static int update_submodule(struct update_data *update_data)

2422 free(remote_ref);

2423 }

2424

2422 - if (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)

2423 - if (run_update_procedure(update_data))

2425 + if (!oideq(&update_data->oid, &update_data->suboid) || update_data->force) {

2426 + int ret;

2427 +

2428 + ret = run_update_procedure(update_data, must_die_on_failure);

2429 + if (*must_die_on_failure)

2430 + return ret;

2431 + if (ret)

2432 return 1;

2433 + }

2434

2435 if (update_data->recursive) {

2436 struct child_process cp = CHILD_PROCESS_INIT;

@@ -2437,14 +2446,13 @@ static int update_submodule(struct update_data *update_data)

2446 prepare_submodule_repo_env(&cp.env);

2447 update_data_to_args(&next, &cp.args);

2448

2440 - /* die() if child process die()'d */

2449 ret = run_command(&cp);

2450 if (!ret)

2451 return 0;

2452 die_message(_("Failed to recurse into submodule path '%s'"),

2453 update_data->displaypath);

2454 if (ret == 128)

2447 - exit(ret);

2455 + *must_die_on_failure = 1;

2456 return ret;

2457 }

2458

@@ -2477,12 +2485,19 @@ static int update_submodules(struct update_data *update_data)

2485

2486 for (i = 0; i < suc.update_clone_nr; i++) {

2487 struct update_clone_data ucd = suc.update_clone[i];

2488 + int must_die_on_failure = 0;

2489 + int code;

2490

2491 oidcpy(&update_data->oid, &ucd.oid);

2492 update_data->just_cloned = ucd.just_cloned;

2493 update_data->sm_path = ucd.sub->path;

2494

2485 - if (update_submodule(update_data))

2495 + code = update_submodule(update_data, &must_die_on_failure);

2496 + if (code)

2497 + ret = code;

2498 + if (must_die_on_failure)

2499 + goto cleanup;

2500 + else if (code)

2501 ret = 1;

2502 }

2503