git-commit: fix double close(2) that can close a wrong file descriptor

The codepath to prepare index files for the temporary and next
index file was closing file descriptor it obtained from the
lockfile API by hand, without letting the API know that the fd
should not be doubly closed.

This is not usually a problem (except it may get EBADFD) but if
we opened another fd for an entirely unrelated purpose (say, an
fd used to mmap a packfile) between the time we close the fd to
the index file and the time we commit or rollback the lockfile
(causing it to also try closing the recorded fd), the lockfile
API will close an incorrect file descriptor that is still used
for an entirely unrelated purpose.

There's four close(fd) calls in prepare_index() and they're all
incorrect.  The open fd's are cleaned up in rollback_index_files() and
shouldn't be closed manually.  The patch below gets rid of the extra
close() calls and should fix the problem.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-commit.c b/builtin-commit.c
index 6d2ca80..16345e9 100644 (file)
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -237,7 +237,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
                int fd = hold_locked_index(&index_lock, 1);
                add_files_to_cache(0, also ? prefix : NULL, pathspec);
                refresh_cache(REFRESH_QUIET);
-               if (write_cache(fd, active_cache, active_nr) || close(fd))
+               if (write_cache(fd, active_cache, active_nr))
                        die("unable to write new_index file");
                commit_style = COMMIT_NORMAL;
                return index_lock.filename;
@@ -256,7 +256,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
                fd = hold_locked_index(&index_lock, 1);
                refresh_cache(REFRESH_QUIET);
                if (write_cache(fd, active_cache, active_nr) ||
-                   close(fd) || commit_locked_index(&index_lock))
+                   commit_locked_index(&index_lock))
                        die("unable to write new_index file");
                commit_style = COMMIT_AS_IS;
                return get_index_file();
@@ -298,7 +298,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
        fd = hold_locked_index(&index_lock, 1);
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
-       if (write_cache(fd, active_cache, active_nr) || close(fd))
+       if (write_cache(fd, active_cache, active_nr))
                die("unable to write new_index file");
 
        fd = hold_lock_file_for_update(&false_lock,
@@ -308,7 +308,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
 
-       if (write_cache(fd, active_cache, active_nr) || close(fd))
+       if (write_cache(fd, active_cache, active_nr))
                die("unable to write temporary index file");
        return false_lock.filename;
 }