Skip to content

Commit 49b6afa

Browse files
committed
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]>
1 parent 721dab7 commit 49b6afa

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
@@ -1339,13 +1339,15 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
13391339
incr = mks_tempfile_m(midx_name.buf, 0444);
13401340
if (!incr) {
13411341
error(_("unable to create temporary MIDX layer"));
1342-
return -1;
1342+
result = -1;
1343+
goto cleanup;
13431344
}
13441345

13451346
if (adjust_shared_perm(r, get_tempfile_path(incr))) {
13461347
error(_("unable to adjust shared permissions for '%s'"),
13471348
get_tempfile_path(incr));
1348-
return -1;
1349+
result = -1;
1350+
goto cleanup;
13491351
}
13501352

13511353
f = hashfd(r->hash_algo, get_tempfile_fd(incr),
@@ -1445,18 +1447,22 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
14451447

14461448
if (!chainf) {
14471449
error_errno(_("unable to open multi-pack-index chain file"));
1448-
return -1;
1450+
result = -1;
1451+
goto cleanup;
14491452
}
14501453

1451-
if (link_midx_to_chain(ctx.base_midx) < 0)
1452-
return -1;
1454+
if (link_midx_to_chain(ctx.base_midx) < 0) {
1455+
result = -1;
1456+
goto cleanup;
1457+
}
14531458

14541459
get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
14551460
object_dir, midx_hash, MIDX_EXT_MIDX);
14561461

14571462
if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
14581463
error_errno(_("unable to rename new multi-pack-index layer"));
1459-
return -1;
1464+
result = -1;
1465+
goto cleanup;
14601466
}
14611467

14621468
strbuf_release(&final_midx_name);

0 commit comments

Comments
 (0)