midx-write: fix leaking hashfile on error cases

When writing the MIDX file we first create the `struct hashfile` used to write the trailer hash, and then afterwards we verify whether we can actually write the MIDX in the first place. When we decide that we can't, this leads to a memory leak because we never free the hash file contents. We could fix this by freeing the hashfile on the exit path. There is a better option though: we can simply move the checks for the error condition earlier. As there is no early exit between creating the hashfile and finalizing it anymore this is sufficient to fix the memory leak. While at it, also move around the block checking for `ctx.entries_nr`. This change is not required to fix the memory leak, but it feels natural to move together all massaging of parameters before we go with them and execute the actual logic. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 22, 2024 at 11:17 UTC 8a7846383e80ad3370344d752c5bb3a2c78cbf65
1 file changed +12 -12
midx-write.c
+12 -12
@@ -1308,6 +1308,18 @@ static int write_midx_internal(const char *object_dir,
1308 pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -
1309 (pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);
1310
1311 + if (ctx.nr - dropped_packs == 0) {
1312 + error(_("no pack files to index."));
1313 + result = 1;
1314 + goto cleanup;
1315 + }
1316 +
1317 + if (!ctx.entries_nr) {
1318 + if (flags & MIDX_WRITE_BITMAP)
1319 + warning(_("refusing to write multi-pack .bitmap without any objects"));
1320 + flags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);
1321 + }
1322 +
1323 if (ctx.incremental) {
1324 struct strbuf lock_name = STRBUF_INIT;
1325
@@ -1333,18 +1345,6 @@ static int write_midx_internal(const char *object_dir,
1345 f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));
1346 }
1347
1336 - if (ctx.nr - dropped_packs == 0) {
1337 - error(_("no pack files to index."));
1338 - result = 1;
1339 - goto cleanup;
1340 - }
1341 -
1342 - if (!ctx.entries_nr) {
1343 - if (flags & MIDX_WRITE_BITMAP)
1344 - warning(_("refusing to write multi-pack .bitmap without any objects"));
1345 - flags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);
1346 - }
1347 -
1348 cf = init_chunkfile(f);
1349
1350 add_chunk(cf, MIDX_CHUNKID_PACKNAMES, pack_name_concat_len,