commit-graph: stop fill_oids_from_packs() progress on error and free()

Fix a bug in fill_oids_from_packs(), we should always stop_progress(), but did not do so if we returned an error here. This also plugs a memory leak in those cases by releasing the two "struct strbuf" variables the function uses. While I'm at it stop hardcoding "-1" here and just use the return value of error() instead, which happens to be "-1". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Mar 4, 2022 at 19:32 UTC 51a94d8ffe57ff40e1db1d5e46a1864a5ded7f49
1 file changed +7 -5
commit-graph.c
+7 -5
@@ -1685,6 +1685,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
1685 struct strbuf progress_title = STRBUF_INIT;
1686 struct strbuf packname = STRBUF_INIT;
1687 int dirlen;
1688 + int ret = 0;
1689
1690 strbuf_addf(&packname, "%s/pack/", ctx->odb->path);
1691 dirlen = packname.len;
@@ -1703,12 +1704,12 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
1704 strbuf_addstr(&packname, pack_indexes->items[i].string);
1705 p = add_packed_git(packname.buf, packname.len, 1);
1706 if (!p) {
1706 - error(_("error adding pack %s"), packname.buf);
1707 - return -1;
1707 + ret = error(_("error adding pack %s"), packname.buf);
1708 + goto cleanup;
1709 }
1710 if (open_pack_index(p)) {
1710 - error(_("error opening index for %s"), packname.buf);
1711 - return -1;
1711 + ret = error(_("error opening index for %s"), packname.buf);
1712 + goto cleanup;
1713 }
1714 for_each_object_in_pack(p, add_packed_commits, ctx,
1715 FOR_EACH_OBJECT_PACK_ORDER);
@@ -1716,11 +1717,12 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
1717 free(p);
1718 }
1719
1720 +cleanup:
1721 stop_progress(&ctx->progress);
1722 strbuf_release(&progress_title);
1723 strbuf_release(&packname);
1724
1723 - return 0;
1725 + return ret;
1726 }
1727
1728 static int fill_oids_from_commits(struct write_commit_graph_context *ctx,