Skip to content

Commit 9c2262d

Browse files
derrickstoleegitster
authored andcommitted
midx-write: use cleanup when incremental midx fails
The incremental mode of writing a multi-pack-index has a few extra conditions that could lead to failure, but these are currently short-ciruiting with 'return -1' instead of setting the method's 'result' variable and going to the cleanup tag. Replace these returns with gotos to avoid memory issues when exiting early due to error conditions. Unfortunately, these error conditions are difficult to reproduce with test cases, which is perhaps one reason why the memory loss was not caught by existing test cases in memory tracking modes. Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3a45c7b commit 9c2262d

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

midx-write.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,13 +1345,15 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
13451345
incr = mks_tempfile_m(midx_name.buf, 0444);
13461346
if (!incr) {
13471347
error(_("unable to create temporary MIDX layer"));
1348-
return -1;
1348+
result = -1;
1349+
goto cleanup;
13491350
}
13501351

13511352
if (adjust_shared_perm(r, get_tempfile_path(incr))) {
13521353
error(_("unable to adjust shared permissions for '%s'"),
13531354
get_tempfile_path(incr));
1354-
return -1;
1355+
result = -1;
1356+
goto cleanup;
13551357
}
13561358

13571359
f = hashfd(r->hash_algo, get_tempfile_fd(incr),
@@ -1451,18 +1453,22 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
14511453

14521454
if (!chainf) {
14531455
error_errno(_("unable to open multi-pack-index chain file"));
1454-
return -1;
1456+
result = -1;
1457+
goto cleanup;
14551458
}
14561459

1457-
if (link_midx_to_chain(ctx.base_midx) < 0)
1458-
return -1;
1460+
if (link_midx_to_chain(ctx.base_midx) < 0) {
1461+
result = -1;
1462+
goto cleanup;
1463+
}
14591464

14601465
get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
14611466
object_dir, midx_hash, MIDX_EXT_MIDX);
14621467

14631468
if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
14641469
error_errno(_("unable to rename new multi-pack-index layer"));
1465-
return -1;
1470+
result = -1;
1471+
goto cleanup;
14661472
}
14671473

14681474
strbuf_release(&final_midx_name);

0 commit comments

Comments
 (0)