From 7db2d08cdccc39ea3d9a1da2da30e54f92b8fd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 14 Jan 2018 17:18:18 +0700 Subject: read-cache.c: change type of "temp" in write_shared_index() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This local variable 'temp' will be passed in from the caller in the next patch. To reduce patch noise, let's change its type now while it's still a local variable and get all the trival conversion out of the next patch. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/read-cache.c b/read-cache.c index 2eb81a6..536086e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2474,30 +2474,32 @@ static int clean_shared_index_files(const char *current_hex) static int write_shared_index(struct index_state *istate, struct lock_file *lock, unsigned flags) { - struct tempfile *temp; + struct tempfile *real_temp; + struct tempfile **temp = &real_temp; struct split_index *si = istate->split_index; int ret; - temp = mks_tempfile(git_path("sharedindex_XXXXXX")); - if (!temp) { + real_temp = mks_tempfile(git_path("sharedindex_XXXXXX")); + if (!real_temp) { hashclr(si->base_sha1); return do_write_locked_index(istate, lock, flags); } + temp = &real_temp; move_cache_to_base_index(istate); - ret = do_write_index(si->base, temp, 1); + ret = do_write_index(si->base, *temp, 1); if (ret) { - delete_tempfile(&temp); + delete_tempfile(temp); return ret; } - ret = adjust_shared_perm(get_tempfile_path(temp)); + ret = adjust_shared_perm(get_tempfile_path(*temp)); if (ret) { int save_errno = errno; - error("cannot fix permission bits on %s", get_tempfile_path(temp)); - delete_tempfile(&temp); + error("cannot fix permission bits on %s", get_tempfile_path(*temp)); + delete_tempfile(temp); errno = save_errno; return ret; } - ret = rename_tempfile(&temp, + ret = rename_tempfile(temp, git_path("sharedindex.%s", sha1_to_hex(si->base->sha1))); if (!ret) { hashcpy(si->base_sha1, si->base->sha1); -- cgit v0.10.2-6-g49f6 From 59f9d2dd60f97c55c92c3273903f64048a27e513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 14 Jan 2018 17:18:19 +0700 Subject: read-cache.c: move tempfile creation/cleanup out of write_shared_index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For one thing, we have more consistent cleanup procedure now and always keep errno intact. The real purpose is the ability to break out of write_locked_index() early when mks_tempfile() fails in the next patch. It's more awkward to do it if this mks_tempfile() is still inside write_shared_index(). Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/read-cache.c b/read-cache.c index 536086e..c568643 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2472,31 +2472,18 @@ static int clean_shared_index_files(const char *current_hex) } static int write_shared_index(struct index_state *istate, - struct lock_file *lock, unsigned flags) + struct tempfile **temp) { - struct tempfile *real_temp; - struct tempfile **temp = &real_temp; struct split_index *si = istate->split_index; int ret; - real_temp = mks_tempfile(git_path("sharedindex_XXXXXX")); - if (!real_temp) { - hashclr(si->base_sha1); - return do_write_locked_index(istate, lock, flags); - } - temp = &real_temp; move_cache_to_base_index(istate); ret = do_write_index(si->base, *temp, 1); - if (ret) { - delete_tempfile(temp); + if (ret) return ret; - } ret = adjust_shared_perm(get_tempfile_path(*temp)); if (ret) { - int save_errno = errno; error("cannot fix permission bits on %s", get_tempfile_path(*temp)); - delete_tempfile(temp); - errno = save_errno; return ret; } ret = rename_tempfile(temp, @@ -2567,7 +2554,21 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock, new_shared_index = istate->cache_changed & SPLIT_INDEX_ORDERED; if (new_shared_index) { - ret = write_shared_index(istate, lock, flags); + struct tempfile *temp; + int saved_errno; + + temp = mks_tempfile(git_path("sharedindex_XXXXXX")); + if (!temp) { + hashclr(si->base_sha1); + ret = do_write_locked_index(istate, lock, flags); + } else + ret = write_shared_index(istate, &temp); + + saved_errno = errno; + if (is_tempfile_active(temp)) + delete_tempfile(&temp); + errno = saved_errno; + if (ret) goto out; } -- cgit v0.10.2-6-g49f6 From ef5b3a6c5e24c54ba4436e225b9431c63ab163f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 24 Jan 2018 16:38:29 +0700 Subject: read-cache: don't write index twice if we can't write shared index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a0a967568e ("update-index --split-index: do not split if $GIT_DIR is read only", 2014-06-13), we tried to make sure we can still write an index, even if the shared index can not be written. We did so by just calling 'do_write_locked_index()' just before 'write_shared_index()'. 'do_write_locked_index()' always at least closes the tempfile nowadays, and used to close or commit the lockfile if COMMIT_LOCK or CLOSE_LOCK were given at the time this feature was introduced. COMMIT_LOCK or CLOSE_LOCK is passed in by most callers of 'write_locked_index()'. After calling 'write_shared_index()', we call 'write_split_index()', which calls 'do_write_locked_index()' again, which then tries to use the closed lockfile again, but in fact fails to do so as it's already closed. This eventually leads to a segfault. Make sure to write the main index only once. [nd: most of the commit message and investigation done by Thomas, I only tweaked the solution a bit] Helped-by: Thomas Gummerer Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/read-cache.c b/read-cache.c index c568643..c58c0a9 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2561,8 +2561,9 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock, if (!temp) { hashclr(si->base_sha1); ret = do_write_locked_index(istate, lock, flags); - } else - ret = write_shared_index(istate, &temp); + goto out; + } + ret = write_shared_index(istate, &temp); saved_errno = errno; if (is_tempfile_active(temp)) diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index af9b847..cbcefa6 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -401,4 +401,23 @@ done <<\EOF 0642 -rw-r---w- EOF +test_expect_success POSIXPERM,SANITY 'graceful handling when splitting index is not allowed' ' + test_create_repo ro && + ( + cd ro && + test_commit initial && + git update-index --split-index && + test -f .git/sharedindex.* + ) && + cp ro/.git/index new-index && + test_when_finished "chmod u+w ro/.git" && + chmod u-w ro/.git && + GIT_INDEX_FILE="$(pwd)/new-index" git -C ro update-index --split-index && + chmod u+w ro/.git && + rm ro/.git/sharedindex.* && + GIT_INDEX_FILE=new-index git ls-files >actual && + echo initial.t >expected && + test_cmp expected actual +' + test_done -- cgit v0.10.2-6-g49f6