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>

Kristian Høgsberg committed Jan 15, 2008 at 15:00 UTC 4439751dcbc3ba2fccae70626654f7950a7b298c

1 file changed +4 -4

builtin-commit.c

+4 -4

@@ -237,7 +237,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)

237 int fd = hold_locked_index(&index_lock, 1);

238 add_files_to_cache(0, also ? prefix : NULL, pathspec);

239 refresh_cache(REFRESH_QUIET);

240 - if (write_cache(fd, active_cache, active_nr) || close(fd))

240 + if (write_cache(fd, active_cache, active_nr))

241 die("unable to write new_index file");

242 commit_style = COMMIT_NORMAL;

243 return index_lock.filename;

@@ -256,7 +256,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)

256 fd = hold_locked_index(&index_lock, 1);

257 refresh_cache(REFRESH_QUIET);

258 if (write_cache(fd, active_cache, active_nr) ||

259 - close(fd) || commit_locked_index(&index_lock))

259 + commit_locked_index(&index_lock))

260 die("unable to write new_index file");

261 commit_style = COMMIT_AS_IS;

262 return get_index_file();

@@ -298,7 +298,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)

298 fd = hold_locked_index(&index_lock, 1);

299 add_remove_files(&partial);

300 refresh_cache(REFRESH_QUIET);

301 - if (write_cache(fd, active_cache, active_nr) || close(fd))

301 + if (write_cache(fd, active_cache, active_nr))

302 die("unable to write new_index file");

303

304 fd = hold_lock_file_for_update(&false_lock,

@@ -308,7 +308,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)

308 add_remove_files(&partial);

309 refresh_cache(REFRESH_QUIET);

310

311 - if (write_cache(fd, active_cache, active_nr) || close(fd))

311 + if (write_cache(fd, active_cache, active_nr))

312 die("unable to write temporary index file");

313 return false_lock.filename;

314 }