write_index_as_tree: cleanup tempfile on error

If we failed to write our new index file, we rollback our lockfile to remove the temporary index. But if we fail before we even get to the write step (because reading the old index failed), we leave the lockfile in place, which makes no sense. In practice this hasn't been a big deal because failing at write_index_as_tree() typically results in the whole program exiting (and thus the tempfile handler kicking in and cleaning up the files). But this function should consistently take responsibility for the resources it allocates. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 5, 2017 at 08:14 UTC c82c75b9518750a94e38c84fc89741a51b815014
1 file changed +15 -8
cache-tree.c
+15 -8
@@ -604,6 +604,7 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co
604 {
605 int entries, was_valid, newfd;
606 struct lock_file *lock_file;
607 + int ret = 0;
608
609 /*
610 * We can't free this memory, it becomes part of a linked list
@@ -614,8 +615,10 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co
615 newfd = hold_lock_file_for_update(lock_file, index_path, LOCK_DIE_ON_ERROR);
616
617 entries = read_index_from(index_state, index_path);
617 - if (entries < 0)
618 - return WRITE_TREE_UNREADABLE_INDEX;
618 + if (entries < 0) {
619 + ret = WRITE_TREE_UNREADABLE_INDEX;
620 + goto out;
621 + }
622 if (flags & WRITE_TREE_IGNORE_CACHE_TREE)
623 cache_tree_free(&index_state->cache_tree);
624
@@ -624,8 +627,10 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co
627
628 was_valid = cache_tree_fully_valid(index_state->cache_tree);
629 if (!was_valid) {
627 - if (cache_tree_update(index_state, flags) < 0)
628 - return WRITE_TREE_UNMERGED_INDEX;
630 + if (cache_tree_update(index_state, flags) < 0) {
631 + ret = WRITE_TREE_UNMERGED_INDEX;
632 + goto out;
633 + }
634 if (0 <= newfd) {
635 if (!write_locked_index(index_state, lock_file, COMMIT_LOCK))
636 newfd = -1;
@@ -641,17 +646,19 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co
646 if (prefix) {
647 struct cache_tree *subtree;
648 subtree = cache_tree_find(index_state->cache_tree, prefix);
644 - if (!subtree)
645 - return WRITE_TREE_PREFIX_ERROR;
649 + if (!subtree) {
650 + ret = WRITE_TREE_PREFIX_ERROR;
651 + goto out;
652 + }
653 hashcpy(sha1, subtree->oid.hash);
654 }
655 else
656 hashcpy(sha1, index_state->cache_tree->oid.hash);
657
658 +out:
659 if (0 <= newfd)
660 rollback_lock_file(lock_file);
653 -
654 - return 0;
661 + return ret;
662 }
663
664 int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)