Skip to content

Commit bf91fd5

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 <stolee@gmail.com>
1 parent da31820 commit bf91fd5

1 file changed

Lines changed: 12 additions & 6 deletions

File tree

midx-write.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,13 +1321,15 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
13211321
incr = mks_tempfile_m(midx_name.buf, 0444);
13221322
if (!incr) {
13231323
error(_("unable to create temporary MIDX layer"));
1324-
return -1;
1324+
result = -1;
1325+
goto cleanup;
13251326
}
13261327

13271328
if (adjust_shared_perm(r, get_tempfile_path(incr))) {
13281329
error(_("unable to adjust shared permissions for '%s'"),
13291330
get_tempfile_path(incr));
1330-
return -1;
1331+
result = -1;
1332+
goto cleanup;
13311333
}
13321334

13331335
f = hashfd(r->hash_algo, get_tempfile_fd(incr),
@@ -1427,18 +1429,22 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
14271429

14281430
if (!chainf) {
14291431
error_errno(_("unable to open multi-pack-index chain file"));
1430-
return -1;
1432+
result = -1;
1433+
goto cleanup;
14311434
}
14321435

1433-
if (link_midx_to_chain(ctx.base_midx) < 0)
1434-
return -1;
1436+
if (link_midx_to_chain(ctx.base_midx) < 0) {
1437+
result = -1;
1438+
goto cleanup;
1439+
}
14351440

14361441
get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
14371442
object_dir, midx_hash, MIDX_EXT_MIDX);
14381443

14391444
if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
14401445
error_errno(_("unable to rename new multi-pack-index layer"));
1441-
return -1;
1446+
result = -1;
1447+
goto cleanup;
14421448
}
14431449

14441450
strbuf_release(&final_midx_name);

0 commit comments

Comments
 (0)