{"thread":{"id":"59062","subject":"[PATCH 0/5] cache API: always have a \"istate->repo\"","startedAt":"2023-01-10T06:17:44Z","lastAt":"2023-01-17T15:34:16Z","messageCount":32,"participants":["Ævar Arnfjörð Bjarmason","Philip Oakley","Jeff Hostetler","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"470042","messageId":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","threadId":"59062","inReplyTo":null,"subject":"[PATCH 0/5] cache API: always have a \"istate->repo\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T06:17:28Z","receivedAt":"2023-01-10T06:17:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"struct index_state\" contains a \"repo\" member, which should be a\npointer to the repository for that index, but which due to us\nconstructing such structs on an ad-hoc basis in various places wasn't\nalways available.\n\nWe'd thus end up with code like this, in the recent\nds/omit-trailing-hash-in-index topic:\n\n\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n\nReally we should be able to trust the \"istate->repo\", but were\ncarrying those sorts of conditionals because our index might come from\na manually constructed source, so we'd have to fall back to\n\"the_repository\".\n\nThis series changes the relvant code so the \"repo\" field is always\nnon-NULL, as 5/5 here shows we had various workarounds in place for\nthat, which can now go away.\n\nSee\nhttps://github.com/avar/git/tree/avar/do-not-lazy-populate-istate-repo\nfor passing CI and a fetchable branch for this topic.\n\nSee https://lore.kernel.org/git/xmqqmt6vqo2w.fsf@gitster.g/ for\nprevious discussion on this topic.\n\nÆvar Arnfjörð Bjarmason (5):\n builtin/difftool.c: { 0 }-initialize rather than using memset()\n sparse-index.c: expand_to_path() can assume non-NULL \"istate\"\n sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()\n read-cache.c: refactor set_new_index_sparsity() for subsequent commit\n treewide: always have a valid \"index_state.repo\" member\n\n apply.c | 2 +-\n builtin/difftool.c | 4 +---\n builtin/sparse-checkout.c | 1 +\n builtin/stash.c | 8 ++++----\n builtin/worktree.c | 2 +-\n fsmonitor-settings.c | 14 --------------\n fsmonitor.c | 2 +-\n merge-recursive.c | 2 +-\n read-cache.c | 23 +++++++++--------------\n repository.c | 2 ++\n revision.c | 2 +-\n sparse-index.c | 15 ++++-----------\n split-index.c | 1 +\n unpack-trees.c | 4 +++-\n 14 files changed, 30 insertions(+), 52 deletions(-)\n\n-- \n2.39.0.1195.gabc92c078c4\n\n"},{"id":"470043","messageId":"patch-1.5-214fe7d3fc2-20230110T060340Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"[PATCH 1/5] builtin/difftool.c: { 0 }-initialize rather than using memset()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T06:17:29Z","receivedAt":"2023-01-10T06:17:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor an initialization of a variable added in\n03831ef7b50 (difftool: implement the functionality in the builtin,\n2017-01-19). This refactoring makes a subsequent change smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/difftool.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex d9b76226f6a..1f9d4324df5 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n-\tstruct index_state wtindex;\n+\tstruct index_state wtindex = { 0 };\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n@@ -387,8 +387,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tmkdir(ldir.buf, 0700);\n \tmkdir(rdir.buf, 0700);\n \n-\tmemset(&wtindex, 0, sizeof(wtindex));\n-\n \tmemset(&lstate, 0, sizeof(lstate));\n \tlstate.base_dir = lbase_dir = xstrdup(ldir.buf);\n \tlstate.base_dir_len = ldir.len;\n-- \n2.39.0.1195.gabc92c078c4\n\n"},{"id":"470044","messageId":"patch-2.5-6db74fe958f-20230110T060340Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"[PATCH 2/5] sparse-index.c: expand_to_path() can assume non-NULL \"istate\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T06:17:30Z","receivedAt":"2023-01-10T06:17:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This function added in [1] was subsequently used in [2]. All of the\ncalls to it are in name-hash.c, and come after calls to\nlazy_init_name_hash(istate). The first thing that function does is:\n\n\tif (istate->name_hash_initialized)\n\t\treturn;\n\nSo we can already assume that we have a non-NULL \"istate\" here, or\nwe'd be segfaulting. Let's not confuse matters by making it appear\nthat's not the case.\n\n1. 71f82d032f3 (sparse-index: expand_to_path(), 2021-04-12)\n2. 4589bca829a (name-hash: use expand_to_path(), 2021-04-12)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sparse-index.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 8c269dab803..65a08d33c73 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -547,7 +547,7 @@ void expand_to_path(struct index_state *istate,\n \tif (in_expand_to_path)\n \t\treturn;\n \n-\tif (!istate || !istate->sparse_index)\n+\tif (!istate->sparse_index)\n \t\treturn;\n \n \tif (!istate->repo)\n-- \n2.39.0.1195.gabc92c078c4\n\n"},{"id":"470045","messageId":"patch-3.5-d96388acef6-20230110T060340Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"[PATCH 3/5] sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T06:17:31Z","receivedAt":"2023-01-10T06:18:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Make the ensure_full_index() function stricter, and have it only\naccept a non-NULL \"struct index_state\". This function (and this\nbehavior) was added in [1].\n\nThe only reason it needed to be this lax was due to interaction with\nrepo_index_has_changes(). See the addition of that code in . This\nfixes one of the TODO comments added in 0c18c059a15, the other one was\nalready removed in [3].\n\nThe other reason for why this was needed dates back to interaction\nwith code added in [4]. In [5] we started calling ensure_full_index()\nin unpack_trees(), but the caller added in 34110cd4e39 wants to pass\nus a NULL \"dst_index\". Let's instead do the NULL check in\nunpack_trees() itself.\n\n1. 4300f8442a2 (sparse-index: implement ensure_full_index(), 2021-03-30)\n2. 0c18c059a15 (read-cache: ensure full index, 2021-04-01)\n3. d76723ee531 (status: use sparse-index throughout, 2021-07-14).\n4. 34110cd4e39 (Make 'unpack_trees()' have a separate source and\n destination index, 2008-03-06)\n5. 6863df35503 (unpack-trees: ensure full index, 2021-03-30)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n read-cache.c | 5 +++--\n sparse-index.c | 4 +++-\n unpack-trees.c | 3 ++-\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex cf87ad70970..9fbff4bc6aa 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2646,8 +2646,7 @@ int repo_index_has_changes(struct repository *repo,\n \t\t}\n \t\tdiff_flush(&opt);\n \t\treturn opt.flags.has_changes != 0;\n-\t} else {\n-\t\t/* TODO: audit for interaction with sparse-index. */\n+\t} else if (istate) {\n \t\tensure_full_index(istate);\n \t\tfor (i = 0; sb && i < istate->cache_nr; i++) {\n \t\t\tif (i)\n@@ -2655,6 +2654,8 @@ int repo_index_has_changes(struct repository *repo,\n \t\t\tstrbuf_addstr(sb, istate->cache[i]->name);\n \t\t}\n \t\treturn !!istate->cache_nr;\n+\t} else {\n+\t\treturn 0;\n \t}\n }\n \ndiff --git a/sparse-index.c b/sparse-index.c\nindex 65a08d33c73..86e3b99870b 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -299,7 +299,7 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \t * If the index is already full, then keep it full. We will convert\n \t * it to a sparse index on write, if possible.\n \t */\n-\tif (!istate || istate->sparse_index == INDEX_EXPANDED)\n+\tif (istate->sparse_index == INDEX_EXPANDED)\n \t\treturn;\n \n \t/*\n@@ -424,6 +424,8 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \n void ensure_full_index(struct index_state *istate)\n {\n+\tif (!istate)\n+\t\tBUG(\"ensure_full_index() must get an index!\");\n \texpand_index(istate, NULL);\n }\n \ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ea09023b015..2381cd7cac4 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1880,7 +1880,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tprepare_repo_settings(repo);\n \tif (repo->settings.command_requires_full_index) {\n \t\tensure_full_index(o->src_index);\n-\t\tensure_full_index(o->dst_index);\n+\t\tif (o->dst_index)\n+\t\t\tensure_full_index(o->dst_index);\n \t}\n \n \tif (o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED &&\n-- \n2.39.0.1195.gabc92c078c4\n\n"},{"id":"470046","messageId":"patch-4.5-e514a881e38-20230110T060340Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"[PATCH 4/5] read-cache.c: refactor set_new_index_sparsity() for subsequent commit","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T06:17:32Z","receivedAt":"2023-01-10T06:18:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor code added to set_new_index_sparsity() in [1] to eliminate\nindentation resulting from putting the body of his function within the\n\"if\" block. Let's instead return early if we have no\nistate->repo. This trivial change makes the subsequent commit's diff\nsmaller.\n\n1. 491df5f679f (read-cache: set sparsity when index is new, 2022-05-10)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n read-cache.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 9fbff4bc6aa..78e38b0da28 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2300,12 +2300,12 @@ static void set_new_index_sparsity(struct index_state *istate)\n \t * If the index's repo exists, mark it sparse according to\n \t * repo settings.\n \t */\n-\tif (istate->repo) {\n-\t\tprepare_repo_settings(istate->repo);\n-\t\tif (!istate->repo->settings.command_requires_full_index &&\n-\t\t is_sparse_index_allowed(istate, 0))\n-\t\t\tistate->sparse_index = 1;\n-\t}\n+\tif (!istate->repo)\n+\t\treturn;\n+\tprepare_repo_settings(istate->repo);\n+\tif (!istate->repo->settings.command_requires_full_index &&\n+\t is_sparse_index_allowed(istate, 0))\n+\t\tistate->sparse_index = 1;\n }\n \n /* remember to discard_cache() before reading a different cache! */\n-- \n2.39.0.1195.gabc92c078c4\n\n"},{"id":"470047","messageId":"patch-5.5-b3b0e592101-20230110T060340Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"[PATCH 5/5] treewide: always have a valid \"index_state.repo\" member","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T06:17:33Z","receivedAt":"2023-01-10T06:18:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the \"repo\" member was added to \"the_index\" in [1] the\nrepo_read_index() was made to populate it, but the unpopulated\n\"the_index\" variable didn't get the same treatment.\n\nLet's do that in initialize_the_repository() when we set it up, and\nlikewise for all of the current callers initialized an empty \"struct\nindex_state\".\n\nThis simplifies code that needs to deal with \"the_index\" or a custom\n\"struct index_state\", we no longer need to second-guess this part of\nthe \"index_state\" deep in the stack. A recent example of such\nsecond-guessing is the \"istate->repo ? istate->repo : the_repository\"\ncode in [2]. We can now simply use \"istate->repo\".\n\nFor \"fsmonitor-settings.c\" we can remove the initialization of a NULL\n\"r\" argument to \"the_repository\". This was added back in [3], and was\nneeded at the time for callers that would pass us the \"r\" from an\n\"istate->repo\". Before this change such a change to\n\"fsmonitor-settings.c\" would segfault all over the test suite (e.g. in\nt0002-gitfile.sh).\n\nThis change has wider eventual implications for\n\"fsmonitor-settings.c\". The reason the other lazy loading behavior in\nit is required (starting with \"if (!r->settings.fsmonitor) ...\" is\nbecause of the previously passed \"r\" being \"NULL\".\n\nI have other local changes on top of this which move its configuration\nreading to \"prepare_repo_settings()\" in \"repo-settings.c\", as we could\nnow start to rely on it being called for our \"r\". But let's leave all\nof that for now, and narrowly remove this particular part of the\nlazy-loading.\n\n1. 1fd9ae517c4 (repository: add repo reference to index_state,\n 2021-01-23)\n2. ee1f0c242ef (read-cache: add index.skipHash config option,\n 2023-01-06)\n3. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,\n 2022-03-25)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n apply.c | 2 +-\n builtin/difftool.c | 2 +-\n builtin/sparse-checkout.c | 1 +\n builtin/stash.c | 8 ++++----\n builtin/worktree.c | 2 +-\n fsmonitor-settings.c | 14 --------------\n fsmonitor.c | 2 +-\n merge-recursive.c | 2 +-\n read-cache.c | 10 ++--------\n repository.c | 2 ++\n revision.c | 2 +-\n sparse-index.c | 9 ---------\n split-index.c | 1 +\n unpack-trees.c | 1 +\n 14 files changed, 17 insertions(+), 41 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 85822280476..47bc6598573 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4105,7 +4105,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p, struct object_id *oid)\n static int build_fake_ancestor(struct apply_state *state, struct patch *list)\n {\n \tstruct patch *patch;\n-\tstruct index_state result = { NULL };\n+\tstruct index_state result = { .repo = state->repo };\n \tstruct lock_file lock = LOCK_INIT;\n \tint res;\n \ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex 1f9d4324df5..ff29906f64a 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n-\tstruct index_state wtindex = { 0 };\n+\tstruct index_state wtindex = { .repo = the_repository };\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 58a22503f04..6f0ea71ebb1 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -217,6 +217,7 @@ static int update_working_directory(struct pattern_list *pl)\n \to.head_idx = -1;\n \to.src_index = r->index;\n \to.dst_index = r->index;\n+\to.result.repo = r;\n \to.skip_sparse_checkout = 0;\n \to.pl = pl;\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex bb0fd861434..a4ff967ea59 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1137,7 +1137,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg,\n \tint ret = 0;\n \tstruct strbuf untracked_msg = STRBUF_INIT;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = { .repo = the_repository };\n \n \tcp_upd_index.git_cmd = 1;\n \tstrvec_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n@@ -1176,7 +1176,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch,\n {\n \tint ret = 0;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = { .repo = the_repository };\n \n \tif (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,\n \t\t\t\t0, NULL)) {\n@@ -1209,7 +1209,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \tint ret = 0;\n \tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = { .repo = the_repository };\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n@@ -1271,7 +1271,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \tstruct rev_info rev;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n \tstruct strbuf diff_output = STRBUF_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = { .repo = the_repository };\n \n \tinit_revisions(&rev, NULL);\n \tcopy_pathspec(&rev.prune_data, ps);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 591d659faea..fd66fb4c165 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -923,7 +923,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \n static void validate_no_submodules(const struct worktree *wt)\n {\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = { .repo = the_repository };\n \tstruct strbuf path = STRBUF_INIT;\n \tint i, found_submodules = 0;\n \ndiff --git a/fsmonitor-settings.c b/fsmonitor-settings.c\nindex ee63a97dc51..899bfe9c813 100644\n--- a/fsmonitor-settings.c\n+++ b/fsmonitor-settings.c\n@@ -143,8 +143,6 @@ static void lookup_fsmonitor_settings(struct repository *r)\n \n enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \n@@ -153,8 +151,6 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)\n \n const char *fsm_settings__get_hook_path(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \n@@ -174,8 +170,6 @@ void fsm_settings__set_ipc(struct repository *r)\n \t * Caller requested IPC explicitly, so avoid (possibly\n \t * recursive) config lookup.\n \t */\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -197,8 +191,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)\n \t * Caller requested hook explicitly, so avoid (possibly\n \t * recursive) config lookup.\n \t */\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -210,8 +202,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)\n \n void fsm_settings__set_disabled(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -223,8 +213,6 @@ void fsm_settings__set_disabled(struct repository *r)\n void fsm_settings__set_incompatible(struct repository *r,\n \t\t\t\t enum fsmonitor_reason reason)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -235,8 +223,6 @@ void fsm_settings__set_incompatible(struct repository *r,\n \n enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 08af00c7387..a5b9e75437b 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -304,7 +304,7 @@ void refresh_fsmonitor(struct index_state *istate)\n \tchar *buf;\n \tunsigned int i;\n \tint is_trivial = 0;\n-\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n+\tstruct repository *r = istate->repo;\n \tenum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);\n \tenum fsmonitor_reason reason = fsm_settings__get_reason(r);\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 2fd0aa96875..a63d2e330d5 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -412,7 +412,7 @@ static int unpack_trees_start(struct merge_options *opt,\n {\n \tint rc;\n \tstruct tree_desc t[3];\n-\tstruct index_state tmp_index = { NULL };\n+\tstruct index_state tmp_index = { .repo = opt->repo };\n \n \tmemset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));\n \tif (opt->priv->call_depth)\ndiff --git a/read-cache.c b/read-cache.c\nindex 78e38b0da28..3f018874926 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2300,8 +2300,6 @@ static void set_new_index_sparsity(struct index_state *istate)\n \t * If the index's repo exists, mark it sparse according to\n \t * repo settings.\n \t */\n-\tif (!istate->repo)\n-\t\treturn;\n \tprepare_repo_settings(istate->repo);\n \tif (!istate->repo->settings.command_requires_full_index &&\n \t is_sparse_index_allowed(istate, 0))\n@@ -2330,8 +2328,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tfd = open(path, O_RDONLY);\n \tif (fd < 0) {\n \t\tif (!must_exist && errno == ENOENT) {\n-\t\t\tif (!istate->repo)\n-\t\t\t\tistate->repo = the_repository;\n \t\t\tset_new_index_sparsity(istate);\n \t\t\treturn 0;\n \t\t}\n@@ -2433,9 +2429,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \ttrace2_data_intmax(\"index\", the_repository, \"read/cache_nr\",\n \t\t\t istate->cache_nr);\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \t/*\n \t * If the command explicitly requires a full index, force it\n \t * to be full. Otherwise, correct the sparsity based on repository\n@@ -2501,6 +2494,7 @@ int read_index_from(struct index_state *istate, const char *path,\n \t\tdiscard_index(split_index->base);\n \telse\n \t\tCALLOC_ARRAY(split_index->base, 1);\n+\tsplit_index->base->repo = istate->repo;\n \n \tbase_oid_hex = oid_to_hex(&split_index->base_oid);\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n@@ -2929,7 +2923,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tint ieot_entries = 1;\n \tstruct index_entry_offset_table *ieot = NULL;\n \tint nr, nr_threads;\n-\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n+\tstruct repository *r = istate->repo;\n \n \tf = hashfd(tempfile->fd, tempfile->filename.buf);\n \ndiff --git a/repository.c b/repository.c\nindex 3427085fd6d..fdebc8c597c 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -28,6 +28,8 @@ void initialize_the_repository(void)\n \tthe_repo.remote_state = remote_state_new();\n \tthe_repo.parsed_objects = parsed_object_pool_new();\n \n+\tthe_index.repo = the_repository;\n+\n \trepo_set_hash_algo(&the_repo, GIT_HASH_SHA1);\n }\n \ndiff --git a/revision.c b/revision.c\nindex 100e5ad5110..785be1eb3bf 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1813,7 +1813,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \tworktrees = get_worktrees();\n \tfor (p = worktrees; *p; p++) {\n \t\tstruct worktree *wt = *p;\n-\t\tstruct index_state istate = { NULL };\n+\t\tstruct index_state istate = { .repo = revs->repo };\n \n \t\tif (wt->is_current)\n \t\t\tcontinue; /* current index already taken care of */\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 86e3b99870b..147a13386a4 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -128,9 +128,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)\n \tif (!core_apply_sparse_checkout || !core_sparse_checkout_cone)\n \t\treturn 0;\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \tif (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {\n \t\tint test_env;\n \n@@ -327,9 +324,6 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \t\t\tpl = NULL;\n \t}\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \t/*\n \t * A NULL pattern set indicates we are expanding a full index, so\n \t * we use a special region name that indicates the full expansion.\n@@ -552,9 +546,6 @@ void expand_to_path(struct index_state *istate,\n \tif (!istate->sparse_index)\n \t\treturn;\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \tin_expand_to_path = 1;\n \n \t/*\ndiff --git a/split-index.c b/split-index.c\nindex 9d0ccc30d00..7815ca9f33e 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -91,6 +91,7 @@ void move_cache_to_base_index(struct index_state *istate)\n \t}\n \n \tCALLOC_ARRAY(si->base, 1);\n+\tsi->base->repo = istate->repo;\n \tsi->base->version = istate->version;\n \t/* zero timestamp disables racy test in ce_write_index() */\n \tsi->base->timestamp = istate->timestamp;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 2381cd7cac4..e8f25bbdce7 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1906,6 +1906,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t}\n \n \tmemset(&o->result, 0, sizeof(o->result));\n+\to->result.repo = o->src_index->repo;\n \to->result.initialized = 1;\n \to->result.timestamp.sec = o->src_index->timestamp.sec;\n \to->result.timestamp.nsec = o->src_index->timestamp.nsec;\n-- \n2.39.0.1195.gabc92c078c4\n\n"},{"id":"470055","messageId":"f2e69f51-ae92-4383-d86d-4d5d29c684a6@iee.email","threadId":"59062","inReplyTo":"patch-3.5-d96388acef6-20230110T060340Z-avarab@gmail.com","subject":"Re: [PATCH 3/5] sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-01-10T10:35:25Z","receivedAt":"2023-01-10T10:35:36Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 10/01/2023 06:17, Ævar Arnfjörð Bjarmason wrote:\n> Make the ensure_full_index() function stricter, and have it only\n> accept a non-NULL \"struct index_state\". This function (and this\n> behavior) was added in [1].\n>\n> The only reason it needed to be this lax was due to interaction with\n> repo_index_has_changes(). See the addition of that code in .\n\nMissing reference. Should this be [2]?\n\n> This\n> fixes one of the TODO comments added in 0c18c059a15, the other one was\n> already removed in [3].\n>\n> The other reason for why this was needed dates back to interaction\n> with code added in [4]. In [5] we started calling ensure_full_index()\n> in unpack_trees(), but the caller added in 34110cd4e39 wants to pass\n> us a NULL \"dst_index\". Let's instead do the NULL check in\n> unpack_trees() itself.\n>\n> 1. 4300f8442a2 (sparse-index: implement ensure_full_index(), 2021-03-30)\n> 2. 0c18c059a15 (read-cache: ensure full index, 2021-04-01)\n> 3. d76723ee531 (status: use sparse-index throughout, 2021-07-14).\n> 4. 34110cd4e39 (Make 'unpack_trees()' have a separate source and\n> destination index, 2008-03-06)\n> 5. 6863df35503 (unpack-trees: ensure full index, 2021-03-30)\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason\n--\nPhilip\n"},{"id":"470058","messageId":"230110.86sfgiziip.gmgdl@evledraar.gmail.com","threadId":"59062","inReplyTo":"f2e69f51-ae92-4383-d86d-4d5d29c684a6@iee.email","subject":"Re: [PATCH 3/5] sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T12:22:09Z","receivedAt":"2023-01-10T12:25:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jan 10 2023, Philip Oakley wrote:\n\n> On 10/01/2023 06:17, Ævar Arnfjörð Bjarmason wrote:\n>> Make the ensure_full_index() function stricter, and have it only\n>> accept a non-NULL \"struct index_state\". This function (and this\n>> behavior) was added in [1].\n>>\n>> The only reason it needed to be this lax was due to interaction with\n>> repo_index_has_changes(). See the addition of that code in .\n>\n> Missing reference. Should this be [2]?\n\nYes, thanks. Will fix that in a re-roll (pending further comments, will\nwait a while).\n"},{"id":"470059","messageId":"230110.86o7r6zibu.gmgdl@evledraar.gmail.com","threadId":"59062","inReplyTo":"patch-5.5-b3b0e592101-20230110T060340Z-avarab@gmail.com","subject":"Re: [PATCH 5/5] treewide: always have a valid \"index_state.repo\" member","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-10T12:24:23Z","receivedAt":"2023-01-10T12:27:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jan 10 2023, Ævar Arnfjörð Bjarmason wrote:\n\n> diff --git a/repository.c b/repository.c\n> index 3427085fd6d..fdebc8c597c 100644\n> --- a/repository.c\n> +++ b/repository.c\n> @@ -28,6 +28,8 @@ void initialize_the_repository(void)\n> \tthe_repo.remote_state = remote_state_new();\n> \tthe_repo.parsed_objects = parsed_object_pool_new();\n> \n> +\tthe_index.repo = the_repository;\n> +\n> \trepo_set_hash_algo(&the_repo, GIT_HASH_SHA1);\n> }\n\nIt's slightly odd to have initialize_the_repository() initialize this\nother variable, I did consider adding a initialize_the_variables(), and\nhave that call an initialize_the_repository() and an\ninitialize_the_index(), but deemed it not worth it for now. Perhaps\nsomething to revisit once we're initializing more variables here.\n\nArguably such a split doesn't even conceptually make sense, as we're now\ndeclaring that \"the_repository\" isn't fully initalized unless the\ncorresponding \"the_index.repo\" is also set, i.e. this really is part of\n\"the_repository\" initialization.\n"},{"id":"470062","messageId":"a28f1f70-0259-e249-87e7-57dedd01de91@jeffhostetler.com","threadId":"59062","inReplyTo":"patch-5.5-b3b0e592101-20230110T060340Z-avarab@gmail.com","subject":"Re: [PATCH 5/5] treewide: always have a valid \"index_state.repo\" member","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2023-01-10T13:13:59Z","receivedAt":"2023-01-10T13:14:05Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 1/10/23 1:17 AM, Ævar Arnfjörð Bjarmason wrote:\n> When the \"repo\" member was added to \"the_index\" in [1] the\n> repo_read_index() was made to populate it, but the unpopulated\n> \"the_index\" variable didn't get the same treatment.\n> \n> Let's do that in initialize_the_repository() when we set it up, and\n> likewise for all of the current callers initialized an empty \"struct\n> index_state\".\n> \n> This simplifies code that needs to deal with \"the_index\" or a custom\n> \"struct index_state\", we no longer need to second-guess this part of\n> the \"index_state\" deep in the stack. A recent example of such\n> second-guessing is the \"istate->repo ? istate->repo : the_repository\"\n> code in [2]. We can now simply use \"istate->repo\".\n> \n> For \"fsmonitor-settings.c\" we can remove the initialization of a NULL\n> \"r\" argument to \"the_repository\". This was added back in [3], and was\n> needed at the time for callers that would pass us the \"r\" from an\n> \"istate->repo\". Before this change such a change to\n> \"fsmonitor-settings.c\" would segfault all over the test suite (e.g. in\n> t0002-gitfile.sh).\n[...]\n\nThanks for looking at this. Yes, it'll be nice to be able to\nfinally depend on istate->repo not being null all over the place.\n\nJeff\n\n"},{"id":"470072","messageId":"14271f92-0ee4-906c-2e12-954ce74fd3ef@github.com","threadId":"59062","inReplyTo":"patch-3.5-d96388acef6-20230110T060340Z-avarab@gmail.com","subject":"Re: [PATCH 3/5] sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-10T14:58:51Z","receivedAt":"2023-01-10T14:59:08Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/10/2023 1:17 AM, Ævar Arnfjörð Bjarmason wrote: \n> The only reason it needed to be this lax was due to interaction with\n> repo_index_has_changes(). See the addition of that code in . This\n> fixes one of the TODO comments added in 0c18c059a15, the other one was\n> already removed in [3].\n\n> 2. 0c18c059a15 (read-cache: ensure full index, 2021-04-01)\n> 3. d76723ee531 (status: use sparse-index throughout, 2021-07-14).\n\n> -\t} else {\n> -\t\t/* TODO: audit for interaction with sparse-index. */\n\nPlease don't drop this comment. It was inserted on purpose before the\n\"ensure_full_index(istate)\" as an indicator that the following loop\nhas not been checked to see if it could be run on a sparse index.\n\nRemoving the comment is like saying \"this loop was checked and we\n_must_ use a full index here\". The case of the TODO being removed in\n[3] was because the loop was audited as being safe on a sparse index\n(and the ensure_full_index() call was removed). I don't believe that\nis what you have done here.\n\n> +\t} else if (istate) {\n> \t\tensure_full_index(istate);\n> \t\tfor (i = 0; sb && i < istate->cache_nr; i++) {\n> \t\t\tif (i)\n\nFurther, this block has all sorts of direct uses of 'istate'\nthat would cause a segfault if 'istate' was NULL. Why do we need\nto check for non-NULL now?\n\nLooking earlier in the function, 'istate' is initialized to\n'repo->index', so the function already assumed the repository had\nan initialized index (or \"tree || !get_oid_tree(\"HEAD\", &cmp)\"\nwas satisfied, which doesn't seem connected to a NULL index).\n\nSo, I'm thinking that we don't actually need any change to\nrepo_index_has_changes() unless it's being called in new ways\nfurther down in the series.\n\nThanks,\n-Stolee\n"},{"id":"470073","messageId":"6b92fad2-6b74-fddb-679c-8c8735e7103d@github.com","threadId":"59062","inReplyTo":"patch-5.5-b3b0e592101-20230110T060340Z-avarab@gmail.com","subject":"Re: [PATCH 5/5] treewide: always have a valid \"index_state.repo\" member","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-10T15:08:53Z","receivedAt":"2023-01-10T15:10:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/10/2023 1:17 AM, Ævar Arnfjörð Bjarmason wrote:\n> When the \"repo\" member was added to \"the_index\" in [1] the\n> repo_read_index() was made to populate it, but the unpopulated\n> \"the_index\" variable didn't get the same treatment.\n> \n> Let's do that in initialize_the_repository() when we set it up, and\n> likewise for all of the current callers initialized an empty \"struct\n> index_state\".\n\n> +\tstruct index_state result = { .repo = state->repo };\n\n> +\tstruct index_state wtindex = { .repo = the_repository };\n\n> +\to.result.repo = r;\n\n> +\tstruct index_state istate = { .repo = the_repository };\n\nI think these initialization updates (along with the others I didn't\ninclude) are satisfactory for now. What worries me is that future\nconsumers that create an index_state will need to remember to manually\ninitialize like this or risk hitting the BUG() statements in\nread-cache.c.\n\nThe only alternative I can think about is to create an initialization\nmethod, say \"init_index_state(struct index_state *, struct repository *)\",\nthat should be called before doing anything with an index_state. This\nincludes running a memset() to clear the struct, making these inline\ninitializers unnecessary.\n\nHowever, I can't decide if that's actually an improvement. I think\nthings tip in favor of the init_index_state() method if there ever\nbecomes another member of struct index_state that _needs_ to be set\nbefore the struct is \"valid\". I doubt that we would add such a thing\nin the near future, so I recommend sticking with this patch as-is.\n\nThanks,\n-Stolee\n"},{"id":"470074","messageId":"84705c15-803f-3062-8a02-474eb1eb053a@github.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"Re: [PATCH 0/5] cache API: always have a \"istate->repo\"","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-10T15:09:54Z","receivedAt":"2023-01-10T15:10:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/10/2023 1:17 AM, Ævar Arnfjörð Bjarmason wrote:\n> The \"struct index_state\" contains a \"repo\" member, which should be a\n> pointer to the repository for that index, but which due to us\n> constructing such structs on an ad-hoc basis in various places wasn't\n> always available.\n> \n> We'd thus end up with code like this, in the recent\n> ds/omit-trailing-hash-in-index topic:\n> \n> \tstruct repository *r = istate->repo ? istate->repo : the_repository;\n> \n> Really we should be able to trust the \"istate->repo\", but were\n> carrying those sorts of conditionals because our index might come from\n> a manually constructed source, so we'd have to fall back to\n> \"the_repository\".\n> \n> This series changes the relvant code so the \"repo\" field is always\n> non-NULL, as 5/5 here shows we had various workarounds in place for\n> that, which can now go away.\n\nThanks for these changes. I only had a recommended change for Patch 3,\nalong with Philip's recommended commit message fixup.\n\nGlad to wrap up this tech debt.\n\nThanks,\n-Stolee\n"},{"id":"470182","messageId":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com","subject":"[PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:22Z","receivedAt":"2023-01-12T12:55:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"struct index_state\" contains a \"repo\" member, which should be a\npointer to the repository for that index, but which due to us\nconstructing such structs on an ad-hoc basis in various places wasn't\nalways available.\n\nWe'd thus end up with code like this, in the recent\nds/omit-trailing-hash-in-index topic:\n\n\t\t\t struct repository *r = istate->repo ? istate->repo : the_repository;\n\nReally we should be able to trust the \"istate->repo\", but were\ncarrying those sorts of conditionals because our index might come from\na manually constructed source, so we'd have to fall back to\n\"the_repository\".\n\nThis series changes the relvant code so the \"repo\" field is always\nnon-NULL, as 6/6 here shows we had various workarounds in place for\nthat, which can now go away.\n\nFor v1 and more general summary, see:\nhttps://lore.kernel.org/git/cover-0.5-00000000000-20230110T060340Z-avarab@gmail.com/\n\nSee\nhttps://github.com/avar/git/tree/avar/do-not-lazy-populate-istate-repo-2\nfor passing CI and a fetchable branch for this topic.\n\nChange since v1:\n\n * Typo/rewording/correcting of commit messages\n * Kept a \"TODO\" comment which Derrick notes I shouldn't have removed\n\n * Derrick suggested in\n https://lore.kernel.org/git/6b92fad2-6b74-fddb-679c-8c8735e7103d@github.com/\n that things might be nicer if we had an explicit initializer, which\n was also the subject of the previous discussion at\n https://lore.kernel.org/git/xmqqmt6vqo2w.fsf@gitster.g/; but\n concluded that it was probably best to leave it for now.\n\n I tried it out, and I think it's worth just doing that now, which\n is why there's a new 5/6 here: We start by adding an\n INDEX_STATE_INIT macro, and corresponding function.\n\n There's a bit of churn in 6/6 as all of those now will take a\n \"repo\" argument, but I think the end result is worth it, because\n even if \"repo\" remains the only thing that we need to initialize\n we're now able to use ALLOC_ARRAY() instead of CALLOC_ARRAY().\n\n We'll thus be helped by analysis tools (which would show access to\n un-init'd memory) if we miss properly init-ing not just the \"repo\"\n field, but anything in the structure, so our test coverage will be\n better.\n\n It also makes the code easier to follow and change, as it's now\n more obvious where we initialize this, and it'll be easier to\n change it in the future if e.g. we add a new member that has\n mandatory initialization (e.g. a \"struct strbuf\").\n\nÆvar Arnfjörð Bjarmason (6):\n builtin/difftool.c: { 0 }-initialize rather than using memset()\n sparse-index.c: expand_to_path() can assume non-NULL \"istate\"\n sparse-index API: BUG() out on NULL ensure_full_index()\n read-cache.c: refactor set_new_index_sparsity() for subsequent commit\n cache API: add a \"INDEX_STATE_INIT\" macro/function, add\n release_index()\n treewide: always have a valid \"index_state.repo\" member\n\n apply.c | 2 +-\n builtin/difftool.c | 4 +---\n builtin/sparse-checkout.c | 1 +\n builtin/stash.c | 16 ++++++-------\n builtin/worktree.c | 2 +-\n cache.h | 16 +++++++++++++\n fsmonitor-settings.c | 14 ------------\n fsmonitor.c | 2 +-\n merge-recursive.c | 2 +-\n read-cache.c | 48 +++++++++++++++++++--------------------\n repository.c | 8 +++++--\n revision.c | 2 +-\n sparse-index.c | 15 ++++--------\n split-index.c | 3 ++-\n unpack-trees.c | 5 ++--\n 15 files changed, 69 insertions(+), 71 deletions(-)\n\nRange-diff against v1:\n1: 214fe7d3fc2 = 1: 214fe7d3fc2 builtin/difftool.c: { 0 }-initialize rather than using memset()\n2: 6db74fe958f = 2: 6db74fe958f sparse-index.c: expand_to_path() can assume non-NULL \"istate\"\n3: d96388acef6 ! 3: 25e9cff0e97 sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - sparse-index API: fix TODO, BUG() out on NULL ensure_full_index()\n + sparse-index API: BUG() out on NULL ensure_full_index()\n \n Make the ensure_full_index() function stricter, and have it only\n accept a non-NULL \"struct index_state\". This function (and this\n behavior) was added in [1].\n \n The only reason it needed to be this lax was due to interaction with\n - repo_index_has_changes(). See the addition of that code in . This\n - fixes one of the TODO comments added in 0c18c059a15, the other one was\n - already removed in [3].\n + repo_index_has_changes(). See the addition of that code in [2].\n \n The other reason for why this was needed dates back to interaction\n - with code added in [4]. In [5] we started calling ensure_full_index()\n + with code added in [3]. In [4] we started calling ensure_full_index()\n in unpack_trees(), but the caller added in 34110cd4e39 wants to pass\n us a NULL \"dst_index\". Let's instead do the NULL check in\n unpack_trees() itself.\n \n 1. 4300f8442a2 (sparse-index: implement ensure_full_index(), 2021-03-30)\n 2. 0c18c059a15 (read-cache: ensure full index, 2021-04-01)\n - 3. d76723ee531 (status: use sparse-index throughout, 2021-07-14).\n - 4. 34110cd4e39 (Make 'unpack_trees()' have a separate source and\n + 3. 34110cd4e39 (Make 'unpack_trees()' have a separate source and\n destination index, 2008-03-06)\n - 5. 6863df35503 (unpack-trees: ensure full index, 2021-03-30)\n + 4. 6863df35503 (unpack-trees: ensure full index, 2021-03-30)\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - ## read-cache.c ##\n -@@ read-cache.c: int repo_index_has_changes(struct repository *repo,\n - \t\t}\n - \t\tdiff_flush(&opt);\n - \t\treturn opt.flags.has_changes != 0;\n --\t} else {\n --\t\t/* TODO: audit for interaction with sparse-index. */\n -+\t} else if (istate) {\n - \t\tensure_full_index(istate);\n - \t\tfor (i = 0; sb && i < istate->cache_nr; i++) {\n - \t\t\tif (i)\n -@@ read-cache.c: int repo_index_has_changes(struct repository *repo,\n - \t\t\tstrbuf_addstr(sb, istate->cache[i]->name);\n - \t\t}\n - \t\treturn !!istate->cache_nr;\n -+\t} else {\n -+\t\treturn 0;\n - \t}\n - }\n - \n -\n ## sparse-index.c ##\n @@ sparse-index.c: void expand_index(struct index_state *istate, struct pattern_list *pl)\n \t * If the index is already full, then keep it full. We will convert\n4: e514a881e38 = 4: a75977bd37b read-cache.c: refactor set_new_index_sparsity() for subsequent commit\n-: ----------- > 5: ae256efe94a cache API: add a \"INDEX_STATE_INIT\" macro/function, add release_index()\n5: b3b0e592101 ! 6: 291233b0420 treewide: always have a valid \"index_state.repo\" member\n @@ Commit message\n second-guessing is the \"istate->repo ? istate->repo : the_repository\"\n code in [2]. We can now simply use \"istate->repo\".\n \n + We're doing this by making use of the INDEX_STATE_INIT() macro (and\n + corresponding function) added in the preceding commit, which now have\n + mandatory \"repo\" arguments. As seen here there are cases where we set\n + it to NULL, but only if we're about to fill in the correct non-NULL\n + \"repo\" right afterwards.\n +\n For \"fsmonitor-settings.c\" we can remove the initialization of a NULL\n \"r\" argument to \"the_repository\". This was added back in [3], and was\n needed at the time for callers that would pass us the \"r\" from an\n @@ apply.c: static int preimage_oid_in_gitlink_patch(struct patch *p, struct object\n static int build_fake_ancestor(struct apply_state *state, struct patch *list)\n {\n \tstruct patch *patch;\n --\tstruct index_state result = { NULL };\n -+\tstruct index_state result = { .repo = state->repo };\n +-\tstruct index_state result = INDEX_STATE_INIT;\n ++\tstruct index_state result = INDEX_STATE_INIT(state->repo);\n \tstruct lock_file lock = LOCK_INIT;\n \tint res;\n \n @@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, co\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n --\tstruct index_state wtindex = { 0 };\n -+\tstruct index_state wtindex = { .repo = the_repository };\n +-\tstruct index_state wtindex = INDEX_STATE_INIT;\n ++\tstruct index_state wtindex = INDEX_STATE_INIT(the_repository);\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n @@ builtin/sparse-checkout.c: static int update_working_directory(struct pattern_li\n \to.head_idx = -1;\n \to.src_index = r->index;\n \to.dst_index = r->index;\n -+\to.result.repo = r;\n +-\tindex_state_init(&o.result);\n ++\tindex_state_init(&o.result, r);\n \to.skip_sparse_checkout = 0;\n \to.pl = pl;\n \n @@ builtin/stash.c: static int save_untracked_files(struct stash_info *info, struct\n \tint ret = 0;\n \tstruct strbuf untracked_msg = STRBUF_INIT;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n --\tstruct index_state istate = { NULL };\n -+\tstruct index_state istate = { .repo = the_repository };\n +-\tstruct index_state istate = INDEX_STATE_INIT;\n ++\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tcp_upd_index.git_cmd = 1;\n \tstrvec_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n @@ builtin/stash.c: static int stash_staged(struct stash_info *info, struct strbuf\n {\n \tint ret = 0;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n --\tstruct index_state istate = { NULL };\n -+\tstruct index_state istate = { .repo = the_repository };\n +-\tstruct index_state istate = INDEX_STATE_INIT;\n ++\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tif (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,\n \t\t\t\t0, NULL)) {\n @@ builtin/stash.c: static int stash_patch(struct stash_info *info, const struct pa\n \tint ret = 0;\n \tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n --\tstruct index_state istate = { NULL };\n -+\tstruct index_state istate = { .repo = the_repository };\n +-\tstruct index_state istate = INDEX_STATE_INIT;\n ++\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n @@ builtin/stash.c: static int stash_working_tree(struct stash_info *info, const st\n \tstruct rev_info rev;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n \tstruct strbuf diff_output = STRBUF_INIT;\n --\tstruct index_state istate = { NULL };\n -+\tstruct index_state istate = { .repo = the_repository };\n +-\tstruct index_state istate = INDEX_STATE_INIT;\n ++\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tinit_revisions(&rev, NULL);\n \tcopy_pathspec(&rev.prune_data, ps);\n @@ builtin/worktree.c: static int unlock_worktree(int ac, const char **av, const ch\n \n static void validate_no_submodules(const struct worktree *wt)\n {\n --\tstruct index_state istate = { NULL };\n -+\tstruct index_state istate = { .repo = the_repository };\n +-\tstruct index_state istate = INDEX_STATE_INIT;\n ++\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tstruct strbuf path = STRBUF_INIT;\n \tint i, found_submodules = 0;\n \n \n + ## cache.h ##\n +@@ cache.h: struct index_state {\n + * If the variable won't be used again, use release_index() to free()\n + * its resources. If it needs to be used again use discard_index(),\n + * which does the same thing, but will use use index_state_init() at\n +- * the end.\n ++ * the end. The discard_index() will use its own \"istate->repo\" as the\n ++ * \"r\" argument to index_state_init() in that case.\n + */\n +-#define INDEX_STATE_INIT { 0 }\n +-void index_state_init(struct index_state *istate);\n ++#define INDEX_STATE_INIT(r) { \\\n ++\t.repo = (r), \\\n ++}\n ++void index_state_init(struct index_state *istate, struct repository *r);\n + void release_index(struct index_state *istate);\n + \n + /* Name hashing */\n +\n ## fsmonitor-settings.c ##\n @@ fsmonitor-settings.c: static void lookup_fsmonitor_settings(struct repository *r)\n \n @@ merge-recursive.c: static int unpack_trees_start(struct merge_options *opt,\n {\n \tint rc;\n \tstruct tree_desc t[3];\n --\tstruct index_state tmp_index = { NULL };\n -+\tstruct index_state tmp_index = { .repo = opt->repo };\n +-\tstruct index_state tmp_index = INDEX_STATE_INIT;\n ++\tstruct index_state tmp_index = INDEX_STATE_INIT(opt->repo);\n \n \tmemset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));\n \tif (opt->priv->call_depth)\n @@ read-cache.c: int do_read_index(struct index_state *istate, const char *path, in\n \t * If the command explicitly requires a full index, force it\n \t * to be full. Otherwise, correct the sparsity based on repository\n @@ read-cache.c: int read_index_from(struct index_state *istate, const char *path,\n - \t\tdiscard_index(split_index->base);\n + \t\trelease_index(split_index->base);\n \telse\n - \t\tCALLOC_ARRAY(split_index->base, 1);\n -+\tsplit_index->base->repo = istate->repo;\n + \t\tALLOC_ARRAY(split_index->base, 1);\n +-\tindex_state_init(split_index->base);\n ++\tindex_state_init(split_index->base, istate->repo);\n \n \tbase_oid_hex = oid_to_hex(&split_index->base_oid);\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n +@@ read-cache.c: int is_index_unborn(struct index_state *istate)\n + \treturn (!istate->cache_nr && !istate->timestamp.sec);\n + }\n + \n +-void index_state_init(struct index_state *istate)\n ++void index_state_init(struct index_state *istate, struct repository *r)\n + {\n +-\tstruct index_state blank = INDEX_STATE_INIT;\n ++\tstruct index_state blank = INDEX_STATE_INIT(r);\n + \tmemcpy(istate, &blank, sizeof(*istate));\n + }\n + \n +@@ read-cache.c: void release_index(struct index_state *istate)\n + void discard_index(struct index_state *istate)\n + {\n + \trelease_index(istate);\n +-\tindex_state_init(istate);\n ++\tindex_state_init(istate, istate->repo);\n + }\n + \n + /*\n @@ read-cache.c: static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tint ieot_entries = 1;\n \tstruct index_entry_offset_table *ieot = NULL;\n @@ repository.c: void initialize_the_repository(void)\n \tthe_repo.remote_state = remote_state_new();\n \tthe_repo.parsed_objects = parsed_object_pool_new();\n \n -+\tthe_index.repo = the_repository;\n ++\tindex_state_init(&the_index, the_repository);\n +\n \trepo_set_hash_algo(&the_repo, GIT_HASH_SHA1);\n }\n \n +@@ repository.c: int repo_read_index(struct repository *repo)\n + \n + \tif (!repo->index) {\n + \t\tALLOC_ARRAY(repo->index, 1);\n +-\t\tindex_state_init(repo->index);\n ++\t\tindex_state_init(repo->index, NULL /* set below */);\n + \t}\n + \n + \t/* Complete the double-reference */\n \n ## revision.c ##\n @@ revision.c: void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \tworktrees = get_worktrees();\n \tfor (p = worktrees; *p; p++) {\n \t\tstruct worktree *wt = *p;\n --\t\tstruct index_state istate = { NULL };\n -+\t\tstruct index_state istate = { .repo = revs->repo };\n +-\t\tstruct index_state istate = INDEX_STATE_INIT;\n ++\t\tstruct index_state istate = INDEX_STATE_INIT(revs->repo);\n \n \t\tif (wt->is_current)\n \t\t\tcontinue; /* current index already taken care of */\n @@ split-index.c\n @@ split-index.c: void move_cache_to_base_index(struct index_state *istate)\n \t}\n \n - \tCALLOC_ARRAY(si->base, 1);\n -+\tsi->base->repo = istate->repo;\n + \tALLOC_ARRAY(si->base, 1);\n +-\tindex_state_init(si->base);\n ++\tindex_state_init(si->base, istate->repo);\n \tsi->base->version = istate->version;\n \t/* zero timestamp disables racy test in ce_write_index() */\n \tsi->base->timestamp = istate->timestamp;\n \n ## unpack-trees.c ##\n @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n + \t\tpopulate_from_existing_patterns(o, &pl);\n \t}\n \n - \tmemset(&o->result, 0, sizeof(o->result));\n -+\to->result.repo = o->src_index->repo;\n +-\tindex_state_init(&o->result);\n ++\tindex_state_init(&o->result, o->src_index->repo);\n \to->result.initialized = 1;\n \to->result.timestamp.sec = o->src_index->timestamp.sec;\n \to->result.timestamp.nsec = o->src_index->timestamp.nsec;\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470183","messageId":"patch-v2-1.6-214fe7d3fc2-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"[PATCH v2 1/6] builtin/difftool.c: { 0 }-initialize rather than using memset()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:23Z","receivedAt":"2023-01-12T12:55:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor an initialization of a variable added in\n03831ef7b50 (difftool: implement the functionality in the builtin,\n2017-01-19). This refactoring makes a subsequent change smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/difftool.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex d9b76226f6a..1f9d4324df5 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n-\tstruct index_state wtindex;\n+\tstruct index_state wtindex = { 0 };\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n@@ -387,8 +387,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tmkdir(ldir.buf, 0700);\n \tmkdir(rdir.buf, 0700);\n \n-\tmemset(&wtindex, 0, sizeof(wtindex));\n-\n \tmemset(&lstate, 0, sizeof(lstate));\n \tlstate.base_dir = lbase_dir = xstrdup(ldir.buf);\n \tlstate.base_dir_len = ldir.len;\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470184","messageId":"patch-v2-2.6-6db74fe958f-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"[PATCH v2 2/6] sparse-index.c: expand_to_path() can assume non-NULL \"istate\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:24Z","receivedAt":"2023-01-12T12:55:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This function added in [1] was subsequently used in [2]. All of the\ncalls to it are in name-hash.c, and come after calls to\nlazy_init_name_hash(istate). The first thing that function does is:\n\n\tif (istate->name_hash_initialized)\n\t\treturn;\n\nSo we can already assume that we have a non-NULL \"istate\" here, or\nwe'd be segfaulting. Let's not confuse matters by making it appear\nthat's not the case.\n\n1. 71f82d032f3 (sparse-index: expand_to_path(), 2021-04-12)\n2. 4589bca829a (name-hash: use expand_to_path(), 2021-04-12)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sparse-index.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 8c269dab803..65a08d33c73 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -547,7 +547,7 @@ void expand_to_path(struct index_state *istate,\n \tif (in_expand_to_path)\n \t\treturn;\n \n-\tif (!istate || !istate->sparse_index)\n+\tif (!istate->sparse_index)\n \t\treturn;\n \n \tif (!istate->repo)\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470185","messageId":"patch-v2-3.6-25e9cff0e97-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"[PATCH v2 3/6] sparse-index API: BUG() out on NULL ensure_full_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:25Z","receivedAt":"2023-01-12T12:55:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Make the ensure_full_index() function stricter, and have it only\naccept a non-NULL \"struct index_state\". This function (and this\nbehavior) was added in [1].\n\nThe only reason it needed to be this lax was due to interaction with\nrepo_index_has_changes(). See the addition of that code in [2].\n\nThe other reason for why this was needed dates back to interaction\nwith code added in [3]. In [4] we started calling ensure_full_index()\nin unpack_trees(), but the caller added in 34110cd4e39 wants to pass\nus a NULL \"dst_index\". Let's instead do the NULL check in\nunpack_trees() itself.\n\n1. 4300f8442a2 (sparse-index: implement ensure_full_index(), 2021-03-30)\n2. 0c18c059a15 (read-cache: ensure full index, 2021-04-01)\n3. 34110cd4e39 (Make 'unpack_trees()' have a separate source and\n destination index, 2008-03-06)\n4. 6863df35503 (unpack-trees: ensure full index, 2021-03-30)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sparse-index.c | 4 +++-\n unpack-trees.c | 3 ++-\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 65a08d33c73..86e3b99870b 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -299,7 +299,7 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \t * If the index is already full, then keep it full. We will convert\n \t * it to a sparse index on write, if possible.\n \t */\n-\tif (!istate || istate->sparse_index == INDEX_EXPANDED)\n+\tif (istate->sparse_index == INDEX_EXPANDED)\n \t\treturn;\n \n \t/*\n@@ -424,6 +424,8 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \n void ensure_full_index(struct index_state *istate)\n {\n+\tif (!istate)\n+\t\tBUG(\"ensure_full_index() must get an index!\");\n \texpand_index(istate, NULL);\n }\n \ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ea09023b015..2381cd7cac4 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1880,7 +1880,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tprepare_repo_settings(repo);\n \tif (repo->settings.command_requires_full_index) {\n \t\tensure_full_index(o->src_index);\n-\t\tensure_full_index(o->dst_index);\n+\t\tif (o->dst_index)\n+\t\t\tensure_full_index(o->dst_index);\n \t}\n \n \tif (o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED &&\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470186","messageId":"patch-v2-4.6-a75977bd37b-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"[PATCH v2 4/6] read-cache.c: refactor set_new_index_sparsity() for subsequent commit","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:26Z","receivedAt":"2023-01-12T12:55:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor code added to set_new_index_sparsity() in [1] to eliminate\nindentation resulting from putting the body of his function within the\n\"if\" block. Let's instead return early if we have no\nistate->repo. This trivial change makes the subsequent commit's diff\nsmaller.\n\n1. 491df5f679f (read-cache: set sparsity when index is new, 2022-05-10)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n read-cache.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex cf87ad70970..25925c8c002 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2300,12 +2300,12 @@ static void set_new_index_sparsity(struct index_state *istate)\n \t * If the index's repo exists, mark it sparse according to\n \t * repo settings.\n \t */\n-\tif (istate->repo) {\n-\t\tprepare_repo_settings(istate->repo);\n-\t\tif (!istate->repo->settings.command_requires_full_index &&\n-\t\t is_sparse_index_allowed(istate, 0))\n-\t\t\tistate->sparse_index = 1;\n-\t}\n+\tif (!istate->repo)\n+\t\treturn;\n+\tprepare_repo_settings(istate->repo);\n+\tif (!istate->repo->settings.command_requires_full_index &&\n+\t is_sparse_index_allowed(istate, 0))\n+\t\tistate->sparse_index = 1;\n }\n \n /* remember to discard_cache() before reading a different cache! */\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470187","messageId":"patch-v2-5.6-ae256efe94a-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"[PATCH v2 5/6] cache API: add a \"INDEX_STATE_INIT\" macro/function, add release_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:27Z","receivedAt":"2023-01-12T12:56:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As we'll see in a subsequent commit we'll get advantages from always\ninitializing the \"repo\" member of the \"struct index_state\". To make\nthat easier let's introduce an initialization macro & function.\n\nThe various ad-hoc initialization of the structure can then be changed\nover to it, and we can remove the various \"0\" assignments in\ndiscard_index() in favor of calling index_state_init() at the end.\n\nWhile not strictly necessary, let's also change the CALLOC_ARRAY() of\nvarious \"struct index_state *\" to use an ALLOC_ARRAY() followed by\nindex_state_init() instead.\n\nWe're then adding the release_index() function and converting some\ncallers (including some of these allocations) over to it if they\neither won't need to use their \"struct index_state\" again, or are just\nabout to call index_state_init().\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n apply.c | 2 +-\n builtin/difftool.c | 2 +-\n builtin/sparse-checkout.c | 1 +\n builtin/stash.c | 16 ++++++++--------\n builtin/worktree.c | 2 +-\n cache.h | 13 +++++++++++++\n merge-recursive.c | 2 +-\n read-cache.c | 31 ++++++++++++++++++-------------\n repository.c | 6 ++++--\n revision.c | 2 +-\n split-index.c | 3 ++-\n unpack-trees.c | 2 +-\n 12 files changed, 52 insertions(+), 30 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 85822280476..821fe411ec8 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4105,7 +4105,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p, struct object_id *oid)\n static int build_fake_ancestor(struct apply_state *state, struct patch *list)\n {\n \tstruct patch *patch;\n-\tstruct index_state result = { NULL };\n+\tstruct index_state result = INDEX_STATE_INIT;\n \tstruct lock_file lock = LOCK_INIT;\n \tint res;\n \ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex 1f9d4324df5..758e7bd3b6a 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n-\tstruct index_state wtindex = { 0 };\n+\tstruct index_state wtindex = INDEX_STATE_INIT;\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 58a22503f04..dc332c6d05f 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -217,6 +217,7 @@ static int update_working_directory(struct pattern_list *pl)\n \to.head_idx = -1;\n \to.src_index = r->index;\n \to.dst_index = r->index;\n+\tindex_state_init(&o.result);\n \to.skip_sparse_checkout = 0;\n \to.pl = pl;\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex bb0fd861434..a5f13fb1e9f 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1137,7 +1137,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg,\n \tint ret = 0;\n \tstruct strbuf untracked_msg = STRBUF_INIT;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = INDEX_STATE_INIT;\n \n \tcp_upd_index.git_cmd = 1;\n \tstrvec_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n@@ -1165,7 +1165,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg,\n \t}\n \n done:\n-\tdiscard_index(&istate);\n+\trelease_index(&istate);\n \tstrbuf_release(&untracked_msg);\n \tremove_path(stash_index_path.buf);\n \treturn ret;\n@@ -1176,7 +1176,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch,\n {\n \tint ret = 0;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = INDEX_STATE_INIT;\n \n \tif (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,\n \t\t\t\t0, NULL)) {\n@@ -1199,7 +1199,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch,\n \t}\n \n done:\n-\tdiscard_index(&istate);\n+\trelease_index(&istate);\n \treturn ret;\n }\n \n@@ -1209,7 +1209,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \tint ret = 0;\n \tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = INDEX_STATE_INIT;\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n@@ -1260,7 +1260,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \t}\n \n done:\n-\tdiscard_index(&istate);\n+\trelease_index(&istate);\n \tremove_path(stash_index_path.buf);\n \treturn ret;\n }\n@@ -1271,7 +1271,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \tstruct rev_info rev;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n \tstruct strbuf diff_output = STRBUF_INIT;\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = INDEX_STATE_INIT;\n \n \tinit_revisions(&rev, NULL);\n \tcopy_pathspec(&rev.prune_data, ps);\n@@ -1319,7 +1319,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \t}\n \n done:\n-\tdiscard_index(&istate);\n+\trelease_index(&istate);\n \trelease_revisions(&rev);\n \tstrbuf_release(&diff_output);\n \tremove_path(stash_index_path.buf);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 591d659faea..311d6e90750 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -923,7 +923,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \n static void validate_no_submodules(const struct worktree *wt)\n {\n-\tstruct index_state istate = { NULL };\n+\tstruct index_state istate = INDEX_STATE_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n \tint i, found_submodules = 0;\n \ndiff --git a/cache.h b/cache.h\nindex 21ed9633b2a..be28fce12ac 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -360,6 +360,19 @@ struct index_state {\n \tstruct pattern_list *sparse_checkout_patterns;\n };\n \n+/**\n+ * A \"struct index_state istate\" must be initialized with\n+ * INDEX_STATE_INIT or the corresponding index_state_init().\n+ *\n+ * If the variable won't be used again, use release_index() to free()\n+ * its resources. If it needs to be used again use discard_index(),\n+ * which does the same thing, but will use use index_state_init() at\n+ * the end.\n+ */\n+#define INDEX_STATE_INIT { 0 }\n+void index_state_init(struct index_state *istate);\n+void release_index(struct index_state *istate);\n+\n /* Name hashing */\n int test_lazy_init_name_hash(struct index_state *istate, int try_threaded);\n void add_name_hash(struct index_state *istate, struct cache_entry *ce);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 2fd0aa96875..0948b3ea961 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -412,7 +412,7 @@ static int unpack_trees_start(struct merge_options *opt,\n {\n \tint rc;\n \tstruct tree_desc t[3];\n-\tstruct index_state tmp_index = { NULL };\n+\tstruct index_state tmp_index = INDEX_STATE_INIT;\n \n \tmemset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));\n \tif (opt->priv->call_depth)\ndiff --git a/read-cache.c b/read-cache.c\nindex 25925c8c002..d191741f600 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2498,9 +2498,10 @@ int read_index_from(struct index_state *istate, const char *path,\n \n \ttrace_performance_enter();\n \tif (split_index->base)\n-\t\tdiscard_index(split_index->base);\n+\t\trelease_index(split_index->base);\n \telse\n-\t\tCALLOC_ARRAY(split_index->base, 1);\n+\t\tALLOC_ARRAY(split_index->base, 1);\n+\tindex_state_init(split_index->base);\n \n \tbase_oid_hex = oid_to_hex(&split_index->base_oid);\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n@@ -2539,7 +2540,13 @@ int is_index_unborn(struct index_state *istate)\n \treturn (!istate->cache_nr && !istate->timestamp.sec);\n }\n \n-void discard_index(struct index_state *istate)\n+void index_state_init(struct index_state *istate)\n+{\n+\tstruct index_state blank = INDEX_STATE_INIT;\n+\tmemcpy(istate, &blank, sizeof(*istate));\n+}\n+\n+void release_index(struct index_state *istate)\n {\n \t/*\n \t * Cache entries in istate->cache[] should have been allocated\n@@ -2551,20 +2558,12 @@ void discard_index(struct index_state *istate)\n \tvalidate_cache_entries(istate);\n \n \tresolve_undo_clear_index(istate);\n-\tistate->cache_nr = 0;\n-\tistate->cache_changed = 0;\n-\tistate->timestamp.sec = 0;\n-\tistate->timestamp.nsec = 0;\n \tfree_name_hash(istate);\n \tcache_tree_free(&(istate->cache_tree));\n-\tistate->initialized = 0;\n-\tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_last_update);\n-\tFREE_AND_NULL(istate->cache);\n-\tistate->cache_alloc = 0;\n+\tfree(istate->fsmonitor_last_update);\n+\tfree(istate->cache);\n \tdiscard_split_index(istate);\n \tfree_untracked_cache(istate->untracked);\n-\tistate->untracked = NULL;\n \n \tif (istate->sparse_checkout_patterns) {\n \t\tclear_pattern_list(istate->sparse_checkout_patterns);\n@@ -2577,6 +2576,12 @@ void discard_index(struct index_state *istate)\n \t}\n }\n \n+void discard_index(struct index_state *istate)\n+{\n+\trelease_index(istate);\n+\tindex_state_init(istate);\n+}\n+\n /*\n * Validate the cache entries of this index.\n * All cache entries associated with this index\ndiff --git a/repository.c b/repository.c\nindex 3427085fd6d..a5805fa33b1 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -302,8 +302,10 @@ int repo_read_index(struct repository *repo)\n {\n \tint res;\n \n-\tif (!repo->index)\n-\t\tCALLOC_ARRAY(repo->index, 1);\n+\tif (!repo->index) {\n+\t\tALLOC_ARRAY(repo->index, 1);\n+\t\tindex_state_init(repo->index);\n+\t}\n \n \t/* Complete the double-reference */\n \tif (!repo->index->repo)\ndiff --git a/revision.c b/revision.c\nindex 100e5ad5110..fb090886f5b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1813,7 +1813,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \tworktrees = get_worktrees();\n \tfor (p = worktrees; *p; p++) {\n \t\tstruct worktree *wt = *p;\n-\t\tstruct index_state istate = { NULL };\n+\t\tstruct index_state istate = INDEX_STATE_INIT;\n \n \t\tif (wt->is_current)\n \t\t\tcontinue; /* current index already taken care of */\ndiff --git a/split-index.c b/split-index.c\nindex 9d0ccc30d00..a5b56c0c37f 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -90,7 +90,8 @@ void move_cache_to_base_index(struct index_state *istate)\n \t\tmem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);\n \t}\n \n-\tCALLOC_ARRAY(si->base, 1);\n+\tALLOC_ARRAY(si->base, 1);\n+\tindex_state_init(si->base);\n \tsi->base->version = istate->version;\n \t/* zero timestamp disables racy test in ce_write_index() */\n \tsi->base->timestamp = istate->timestamp;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 2381cd7cac4..d1d1b0f319e 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1905,7 +1905,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\tpopulate_from_existing_patterns(o, &pl);\n \t}\n \n-\tmemset(&o->result, 0, sizeof(o->result));\n+\tindex_state_init(&o->result);\n \to->result.initialized = 1;\n \to->result.timestamp.sec = o->src_index->timestamp.sec;\n \to->result.timestamp.nsec = o->src_index->timestamp.nsec;\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470188","messageId":"patch-v2-6.6-291233b0420-20230112T124842Z-avarab@gmail.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"[PATCH v2 6/6] treewide: always have a valid \"index_state.repo\" member","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T12:55:28Z","receivedAt":"2023-01-12T12:56:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the \"repo\" member was added to \"the_index\" in [1] the\nrepo_read_index() was made to populate it, but the unpopulated\n\"the_index\" variable didn't get the same treatment.\n\nLet's do that in initialize_the_repository() when we set it up, and\nlikewise for all of the current callers initialized an empty \"struct\nindex_state\".\n\nThis simplifies code that needs to deal with \"the_index\" or a custom\n\"struct index_state\", we no longer need to second-guess this part of\nthe \"index_state\" deep in the stack. A recent example of such\nsecond-guessing is the \"istate->repo ? istate->repo : the_repository\"\ncode in [2]. We can now simply use \"istate->repo\".\n\nWe're doing this by making use of the INDEX_STATE_INIT() macro (and\ncorresponding function) added in the preceding commit, which now have\nmandatory \"repo\" arguments. As seen here there are cases where we set\nit to NULL, but only if we're about to fill in the correct non-NULL\n\"repo\" right afterwards.\n\nFor \"fsmonitor-settings.c\" we can remove the initialization of a NULL\n\"r\" argument to \"the_repository\". This was added back in [3], and was\nneeded at the time for callers that would pass us the \"r\" from an\n\"istate->repo\". Before this change such a change to\n\"fsmonitor-settings.c\" would segfault all over the test suite (e.g. in\nt0002-gitfile.sh).\n\nThis change has wider eventual implications for\n\"fsmonitor-settings.c\". The reason the other lazy loading behavior in\nit is required (starting with \"if (!r->settings.fsmonitor) ...\" is\nbecause of the previously passed \"r\" being \"NULL\".\n\nI have other local changes on top of this which move its configuration\nreading to \"prepare_repo_settings()\" in \"repo-settings.c\", as we could\nnow start to rely on it being called for our \"r\". But let's leave all\nof that for now, and narrowly remove this particular part of the\nlazy-loading.\n\n1. 1fd9ae517c4 (repository: add repo reference to index_state,\n 2021-01-23)\n2. ee1f0c242ef (read-cache: add index.skipHash config option,\n 2023-01-06)\n3. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,\n 2022-03-25)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n apply.c | 2 +-\n builtin/difftool.c | 2 +-\n builtin/sparse-checkout.c | 2 +-\n builtin/stash.c | 8 ++++----\n builtin/worktree.c | 2 +-\n cache.h | 9 ++++++---\n fsmonitor-settings.c | 14 --------------\n fsmonitor.c | 2 +-\n merge-recursive.c | 2 +-\n read-cache.c | 17 +++++------------\n repository.c | 4 +++-\n revision.c | 2 +-\n sparse-index.c | 9 ---------\n split-index.c | 2 +-\n unpack-trees.c | 2 +-\n 15 files changed, 27 insertions(+), 52 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 821fe411ec8..5eec433583e 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4105,7 +4105,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p, struct object_id *oid)\n static int build_fake_ancestor(struct apply_state *state, struct patch *list)\n {\n \tstruct patch *patch;\n-\tstruct index_state result = INDEX_STATE_INIT;\n+\tstruct index_state result = INDEX_STATE_INIT(state->repo);\n \tstruct lock_file lock = LOCK_INIT;\n \tint res;\n \ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex 758e7bd3b6a..dbbfb19f192 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n-\tstruct index_state wtindex = INDEX_STATE_INIT;\n+\tstruct index_state wtindex = INDEX_STATE_INIT(the_repository);\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex dc332c6d05f..c3738154918 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -217,7 +217,7 @@ static int update_working_directory(struct pattern_list *pl)\n \to.head_idx = -1;\n \to.src_index = r->index;\n \to.dst_index = r->index;\n-\tindex_state_init(&o.result);\n+\tindex_state_init(&o.result, r);\n \to.skip_sparse_checkout = 0;\n \to.pl = pl;\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex a5f13fb1e9f..839569a9803 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1137,7 +1137,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg,\n \tint ret = 0;\n \tstruct strbuf untracked_msg = STRBUF_INIT;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tcp_upd_index.git_cmd = 1;\n \tstrvec_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n@@ -1176,7 +1176,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch,\n {\n \tint ret = 0;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tif (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,\n \t\t\t\t0, NULL)) {\n@@ -1209,7 +1209,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \tint ret = 0;\n \tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n@@ -1271,7 +1271,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \tstruct rev_info rev;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n \tstruct strbuf diff_output = STRBUF_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tinit_revisions(&rev, NULL);\n \tcopy_pathspec(&rev.prune_data, ps);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 311d6e90750..f51c40f1e1e 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -923,7 +923,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \n static void validate_no_submodules(const struct worktree *wt)\n {\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tstruct strbuf path = STRBUF_INIT;\n \tint i, found_submodules = 0;\n \ndiff --git a/cache.h b/cache.h\nindex be28fce12ac..4bf14e0bd94 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -367,10 +367,13 @@ struct index_state {\n * If the variable won't be used again, use release_index() to free()\n * its resources. If it needs to be used again use discard_index(),\n * which does the same thing, but will use use index_state_init() at\n- * the end.\n+ * the end. The discard_index() will use its own \"istate->repo\" as the\n+ * \"r\" argument to index_state_init() in that case.\n */\n-#define INDEX_STATE_INIT { 0 }\n-void index_state_init(struct index_state *istate);\n+#define INDEX_STATE_INIT(r) { \\\n+\t.repo = (r), \\\n+}\n+void index_state_init(struct index_state *istate, struct repository *r);\n void release_index(struct index_state *istate);\n \n /* Name hashing */\ndiff --git a/fsmonitor-settings.c b/fsmonitor-settings.c\nindex ee63a97dc51..899bfe9c813 100644\n--- a/fsmonitor-settings.c\n+++ b/fsmonitor-settings.c\n@@ -143,8 +143,6 @@ static void lookup_fsmonitor_settings(struct repository *r)\n \n enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \n@@ -153,8 +151,6 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)\n \n const char *fsm_settings__get_hook_path(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \n@@ -174,8 +170,6 @@ void fsm_settings__set_ipc(struct repository *r)\n \t * Caller requested IPC explicitly, so avoid (possibly\n \t * recursive) config lookup.\n \t */\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -197,8 +191,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)\n \t * Caller requested hook explicitly, so avoid (possibly\n \t * recursive) config lookup.\n \t */\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -210,8 +202,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)\n \n void fsm_settings__set_disabled(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -223,8 +213,6 @@ void fsm_settings__set_disabled(struct repository *r)\n void fsm_settings__set_incompatible(struct repository *r,\n \t\t\t\t enum fsmonitor_reason reason)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -235,8 +223,6 @@ void fsm_settings__set_incompatible(struct repository *r,\n \n enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 08af00c7387..a5b9e75437b 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -304,7 +304,7 @@ void refresh_fsmonitor(struct index_state *istate)\n \tchar *buf;\n \tunsigned int i;\n \tint is_trivial = 0;\n-\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n+\tstruct repository *r = istate->repo;\n \tenum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);\n \tenum fsmonitor_reason reason = fsm_settings__get_reason(r);\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 0948b3ea961..ae469f8cc81 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -412,7 +412,7 @@ static int unpack_trees_start(struct merge_options *opt,\n {\n \tint rc;\n \tstruct tree_desc t[3];\n-\tstruct index_state tmp_index = INDEX_STATE_INIT;\n+\tstruct index_state tmp_index = INDEX_STATE_INIT(opt->repo);\n \n \tmemset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));\n \tif (opt->priv->call_depth)\ndiff --git a/read-cache.c b/read-cache.c\nindex d191741f600..7bd12afb38c 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2300,8 +2300,6 @@ static void set_new_index_sparsity(struct index_state *istate)\n \t * If the index's repo exists, mark it sparse according to\n \t * repo settings.\n \t */\n-\tif (!istate->repo)\n-\t\treturn;\n \tprepare_repo_settings(istate->repo);\n \tif (!istate->repo->settings.command_requires_full_index &&\n \t is_sparse_index_allowed(istate, 0))\n@@ -2330,8 +2328,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tfd = open(path, O_RDONLY);\n \tif (fd < 0) {\n \t\tif (!must_exist && errno == ENOENT) {\n-\t\t\tif (!istate->repo)\n-\t\t\t\tistate->repo = the_repository;\n \t\t\tset_new_index_sparsity(istate);\n \t\t\treturn 0;\n \t\t}\n@@ -2433,9 +2429,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \ttrace2_data_intmax(\"index\", the_repository, \"read/cache_nr\",\n \t\t\t istate->cache_nr);\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \t/*\n \t * If the command explicitly requires a full index, force it\n \t * to be full. Otherwise, correct the sparsity based on repository\n@@ -2501,7 +2494,7 @@ int read_index_from(struct index_state *istate, const char *path,\n \t\trelease_index(split_index->base);\n \telse\n \t\tALLOC_ARRAY(split_index->base, 1);\n-\tindex_state_init(split_index->base);\n+\tindex_state_init(split_index->base, istate->repo);\n \n \tbase_oid_hex = oid_to_hex(&split_index->base_oid);\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n@@ -2540,9 +2533,9 @@ int is_index_unborn(struct index_state *istate)\n \treturn (!istate->cache_nr && !istate->timestamp.sec);\n }\n \n-void index_state_init(struct index_state *istate)\n+void index_state_init(struct index_state *istate, struct repository *r)\n {\n-\tstruct index_state blank = INDEX_STATE_INIT;\n+\tstruct index_state blank = INDEX_STATE_INIT(r);\n \tmemcpy(istate, &blank, sizeof(*istate));\n }\n \n@@ -2579,7 +2572,7 @@ void release_index(struct index_state *istate)\n void discard_index(struct index_state *istate)\n {\n \trelease_index(istate);\n-\tindex_state_init(istate);\n+\tindex_state_init(istate, istate->repo);\n }\n \n /*\n@@ -2933,7 +2926,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tint ieot_entries = 1;\n \tstruct index_entry_offset_table *ieot = NULL;\n \tint nr, nr_threads;\n-\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n+\tstruct repository *r = istate->repo;\n \n \tf = hashfd(tempfile->fd, tempfile->filename.buf);\n \ndiff --git a/repository.c b/repository.c\nindex a5805fa33b1..0e15220e6ac 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -28,6 +28,8 @@ void initialize_the_repository(void)\n \tthe_repo.remote_state = remote_state_new();\n \tthe_repo.parsed_objects = parsed_object_pool_new();\n \n+\tindex_state_init(&the_index, the_repository);\n+\n \trepo_set_hash_algo(&the_repo, GIT_HASH_SHA1);\n }\n \n@@ -304,7 +306,7 @@ int repo_read_index(struct repository *repo)\n \n \tif (!repo->index) {\n \t\tALLOC_ARRAY(repo->index, 1);\n-\t\tindex_state_init(repo->index);\n+\t\tindex_state_init(repo->index, NULL /* set below */);\n \t}\n \n \t/* Complete the double-reference */\ndiff --git a/revision.c b/revision.c\nindex fb090886f5b..21f5f572c22 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1813,7 +1813,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \tworktrees = get_worktrees();\n \tfor (p = worktrees; *p; p++) {\n \t\tstruct worktree *wt = *p;\n-\t\tstruct index_state istate = INDEX_STATE_INIT;\n+\t\tstruct index_state istate = INDEX_STATE_INIT(revs->repo);\n \n \t\tif (wt->is_current)\n \t\t\tcontinue; /* current index already taken care of */\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 86e3b99870b..147a13386a4 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -128,9 +128,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)\n \tif (!core_apply_sparse_checkout || !core_sparse_checkout_cone)\n \t\treturn 0;\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \tif (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {\n \t\tint test_env;\n \n@@ -327,9 +324,6 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \t\t\tpl = NULL;\n \t}\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \t/*\n \t * A NULL pattern set indicates we are expanding a full index, so\n \t * we use a special region name that indicates the full expansion.\n@@ -552,9 +546,6 @@ void expand_to_path(struct index_state *istate,\n \tif (!istate->sparse_index)\n \t\treturn;\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \tin_expand_to_path = 1;\n \n \t/*\ndiff --git a/split-index.c b/split-index.c\nindex a5b56c0c37f..5d0f04763ea 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -91,7 +91,7 @@ void move_cache_to_base_index(struct index_state *istate)\n \t}\n \n \tALLOC_ARRAY(si->base, 1);\n-\tindex_state_init(si->base);\n+\tindex_state_init(si->base, istate->repo);\n \tsi->base->version = istate->version;\n \t/* zero timestamp disables racy test in ce_write_index() */\n \tsi->base->timestamp = istate->timestamp;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex d1d1b0f319e..3d05e45a279 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1905,7 +1905,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\tpopulate_from_existing_patterns(o, &pl);\n \t}\n \n-\tindex_state_init(&o->result);\n+\tindex_state_init(&o->result, o->src_index->repo);\n \to->result.initialized = 1;\n \to->result.timestamp.sec = o->src_index->timestamp.sec;\n \to->result.timestamp.nsec = o->src_index->timestamp.nsec;\n-- \n2.39.0.1205.g2ca064edc27\n\n"},{"id":"470200","messageId":"270ef93c-cfdc-d119-5740-f704d7f2a029@github.com","threadId":"59062","inReplyTo":"patch-v2-5.6-ae256efe94a-20230112T124842Z-avarab@gmail.com","subject":"Re: [PATCH v2 5/6] cache API: add a \"INDEX_STATE_INIT\" macro/function, add release_index()","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-12T15:21:15Z","receivedAt":"2023-01-12T15:29:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:\n> As we'll see in a subsequent commit we'll get advantages from always\n> initializing the \"repo\" member of the \"struct index_state\". To make\n> that easier let's introduce an initialization macro & function.\n> \n> The various ad-hoc initialization of the structure can then be changed\n> over to it, and we can remove the various \"0\" assignments in\n> discard_index() in favor of calling index_state_init() at the end.\n\n> -\tmemset(&o->result, 0, sizeof(o->result));\n> +\tindex_state_init(&o->result);\n> \to->result.initialized = 1;\n\nIt's interesting that 'struct index_state' has an 'initialized'\nmember that we aren't setting in index_state_init(). Perhaps it's\nonly being used in special cases like this, and means something\nmore specific than \"index_state_init() was run\"? Or maybe we\ncould add it to INDEX_STATE_INIT and drop this line?\n\nThanks,\n-Stolee\n"},{"id":"470201","messageId":"f55958e0-110b-3386-b82a-154d02c71696@github.com","threadId":"59062","inReplyTo":"patch-v2-6.6-291233b0420-20230112T124842Z-avarab@gmail.com","subject":"Re: [PATCH v2 6/6] treewide: always have a valid \"index_state.repo\" member","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-12T15:22:29Z","receivedAt":"2023-01-12T15:31:14Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:\n\n> We're doing this by making use of the INDEX_STATE_INIT() macro (and\n> corresponding function) added in the preceding commit, which now have\n> mandatory \"repo\" arguments. As seen here there are cases where we set\n> it to NULL, but only if we're about to fill in the correct non-NULL\n> \"repo\" right afterwards.\n\nThe changes in part 5 to depend on INDEX_STATE_INIT and\nindex_state_init() make the changes in this patch much more\nmechanical and easy to follow.\n\nThanks,\n-Stolee\n"},{"id":"470202","messageId":"26708e4a-93bf-5dec-2b3e-da8f4ce37571@github.com","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-12T15:23:54Z","receivedAt":"2023-01-12T15:32:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:\n\n> * Derrick suggested in\n> https://lore.kernel.org/git/6b92fad2-6b74-fddb-679c-8c8735e7103d@github.com/\n> that things might be nicer if we had an explicit initializer, which\n> was also the subject of the previous discussion at\n> https://lore.kernel.org/git/xmqqmt6vqo2w.fsf@gitster.g/; but\n> concluded that it was probably best to leave it for now.\n> \n> I tried it out, and I think it's worth just doing that now, which\n> is why there's a new 5/6 here: We start by adding an\n> INDEX_STATE_INIT macro, and corresponding function.\n> \n> There's a bit of churn in 6/6 as all of those now will take a\n> \"repo\" argument, but I think the end result is worth it, because\n> even if \"repo\" remains the only thing that we need to initialize\n> we're now able to use ALLOC_ARRAY() instead of CALLOC_ARRAY().\n> \n> We'll thus be helped by analysis tools (which would show access to\n> un-init'd memory) if we miss properly init-ing not just the \"repo\"\n> field, but anything in the structure, so our test coverage will be\n> better.\n> \n> It also makes the code easier to follow and change, as it's now\n> more obvious where we initialize this, and it'll be easier to\n> change it in the future if e.g. we add a new member that has\n> mandatory initialization (e.g. a \"struct strbuf\").\n\nI wasn't expecting the initializer idea to work as well as it has.\nNow, Patch 5 does all the heavy lifting and Patch 6 is an easy read.\n\nOutside of one question about the istate->initialized member (which\nmight not need any change) I'm happy with this version.\n\nThanks,\n-Stolee\n"},{"id":"470208","messageId":"230112.86zganzpwk.gmgdl@evledraar.gmail.com","threadId":"59062","inReplyTo":"270ef93c-cfdc-d119-5740-f704d7f2a029@github.com","subject":"Re: [PATCH v2 5/6] cache API: add a \"INDEX_STATE_INIT\" macro/function, add release_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T16:19:20Z","receivedAt":"2023-01-12T16:23:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jan 12 2023, Derrick Stolee wrote:\n\n> On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:\n>> As we'll see in a subsequent commit we'll get advantages from always\n>> initializing the \"repo\" member of the \"struct index_state\". To make\n>> that easier let's introduce an initialization macro & function.\n>> \n>> The various ad-hoc initialization of the structure can then be changed\n>> over to it, and we can remove the various \"0\" assignments in\n>> discard_index() in favor of calling index_state_init() at the end.\n>\n>> -\tmemset(&o->result, 0, sizeof(o->result));\n>> +\tindex_state_init(&o->result);\n>> \to->result.initialized = 1;\n>\n> It's interesting that 'struct index_state' has an 'initialized'\n> member that we aren't setting in index_state_init(). Perhaps it's\n> only being used in special cases like this, and means something\n> more specific than \"index_state_init() was run\"? Or maybe we\n> could add it to INDEX_STATE_INIT and drop this line?\n\nIt's unrelated, and doing that would be a bug. It's a bit unfortunately\nnamed, a better name might be \"read_index_data\" or something.\n\nIt was added in 913e0e99b6a (unpack_trees(): protect the handcrafted\nin-core index from read_cache(), 2008-08-23), which shows the use-case,\ni.e. it's for avoiding re-reading the index file itself (or in that\ncase, to trust our hand-crafted faked-up version of it).\n\nI opted not to mention it in the commit message, after being\nsufficiently convinced that it was unrelated, which was probably a\nmistake :)\n\nJust as a sanity check, we do have really good test coverage of the\ndifference, at least 1/2 of the tests I bothered to wait for failed when\nI tried this on top:\n\ndiff --git a/cache.h b/cache.h\nindex 4bf14e0bd94..1f8e5f4e823 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -371,6 +371,7 @@ struct index_state {\n * \"r\" argument to index_state_init() in that case.\n */\n #define INDEX_STATE_INIT(r) { \\\n+\t.initialized = 1, \\\n \t.repo = (r), \\\n }\n void index_state_init(struct index_state *istate, struct repository *r);\n"},{"id":"470216","messageId":"ee711878-87d7-dc94-9913-681cbd6b67a5@github.com","threadId":"59062","inReplyTo":"230112.86zganzpwk.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 5/6] cache API: add a \"INDEX_STATE_INIT\" macro/function, add release_index()","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-12T16:47:35Z","receivedAt":"2023-01-12T17:14:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/12/2023 11:19 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Jan 12 2023, Derrick Stolee wrote:\n>> It's interesting that 'struct index_state' has an 'initialized'\n>> member that we aren't setting in index_state_init(). Perhaps it's\n>> only being used in special cases like this, and means something\n>> more specific than \"index_state_init() was run\"? Or maybe we\n>> could add it to INDEX_STATE_INIT and drop this line?\n> \n> It's unrelated, and doing that would be a bug. It's a bit unfortunately\n> named, a better name might be \"read_index_data\" or something.\n> \n> It was added in 913e0e99b6a (unpack_trees(): protect the handcrafted\n> in-core index from read_cache(), 2008-08-23), which shows the use-case,\n> i.e. it's for avoiding re-reading the index file itself (or in that\n> case, to trust our hand-crafted faked-up version of it).\n> \n> I opted not to mention it in the commit message, after being\n> sufficiently convinced that it was unrelated, which was probably a\n> mistake :)\n> \n> Just as a sanity check, we do have really good test coverage of the\n> difference, at least 1/2 of the tests I bothered to wait for failed when\n> I tried this on top:\n> \n> diff --git a/cache.h b/cache.h\n> index 4bf14e0bd94..1f8e5f4e823 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -371,6 +371,7 @@ struct index_state {\n> * \"r\" argument to index_state_init() in that case.\n> */\n> #define INDEX_STATE_INIT(r) { \\\n> +\t.initialized = 1, \\\n> \t.repo = (r), \\\n\nThanks for the extra info. The patch is clearly correct with that\ninformation.\n\nThanks,\n-Stolee\n"},{"id":"470298","messageId":"xmqq7cxq47aw.fsf@gitster.g","threadId":"59062","inReplyTo":"26708e4a-93bf-5dec-2b3e-da8f4ce37571@github.com","subject":"Re: [PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-13T18:29:27Z","receivedAt":"2023-01-13T18:34:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> I wasn't expecting the initializer idea to work as well as it has.\n> Now, Patch 5 does all the heavy lifting and Patch 6 is an easy read.\n>\n> Outside of one question about the istate->initialized member (which\n> might not need any change) I'm happy with this version.\n\nThanks, both. And especially for finding 913e0e99 (unpack_trees():\nprotect the handcrafted in-core index from read_cache(),\n2008-08-23).\n\nWill queue.\n"},{"id":"470299","messageId":"xmqqtu0u2q9u.fsf@gitster.g","threadId":"59062","inReplyTo":"cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-13T19:22:37Z","receivedAt":"2023-01-13T19:22:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The \"struct index_state\" contains a \"repo\" member, which should be a\n> pointer to the repository for that index, but which due to us\n> constructing such structs on an ad-hoc basis in various places wasn't\n> always available.\n\nI'd exclude 6/6 for now, as it seems to depend on some changes only\nin 'next'. Feel free to resend only that step with reduced scope so\nthat it applies to 'master', and send in incremental updates when\neach of these topics that are only in 'next' graduates.\n\nThanks.\n"},{"id":"470432","messageId":"230116.86a62izjg5.gmgdl@evledraar.gmail.com","threadId":"59062","inReplyTo":"xmqqtu0u2q9u.fsf@gitster.g","subject":"Re: [PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-16T13:38:31Z","receivedAt":"2023-01-16T13:40:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jan 13 2023, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> The \"struct index_state\" contains a \"repo\" member, which should be a\n>> pointer to the repository for that index, but which due to us\n>> constructing such structs on an ad-hoc basis in various places wasn't\n>> always available.\n>\n> I'd exclude 6/6 for now, as it seems to depend on some changes only\n> in 'next'. Feel free to resend only that step with reduced scope so\n> that it applies to 'master', and send in incremental updates when\n> each of these topics that are only in 'next' graduates.\n\nOkey, the 6/6 requires ds/omit-trailing-hash-in-index. As both it and\nthe 1-5/6 of this are in \"next\" now I think it's best that I just submit\nthe 6/6 stand-alone after both of those have graduated.\n\n"},{"id":"470439","messageId":"b2eca967-4c52-3f8d-f3fe-b1ed41b06280@iee.email","threadId":"59062","inReplyTo":"230116.86a62izjg5.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-01-16T16:53:41Z","receivedAt":"2023-01-16T17:14:38Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 16/01/2023 13:38, Ævar Arnfjörð Bjarmason wrote:\n> On Fri, Jan 13 2023, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> The \"struct index_state\" contains a \"repo\" member, which should be a\n>>> pointer to the repository for that index, but which due to us\n>>> constructing such structs on an ad-hoc basis in various places wasn't\n>>> always available.\n>> I'd exclude 6/6 for now, as it seems to depend on some changes only\n>> in 'next'. Feel free to resend only that step with reduced scope so\n>> that it applies to 'master', and send in incremental updates when\n>> each of these topics that are only in 'next' graduates.\n> Okey, the 6/6 requires ds/omit-trailing-hash-in-index. As both it and\n> the 1-5/6 of this are in \"next\" now I think it's best that I just submit\n> the 6/6 stand-alone after both of those have graduated.\n>\nmicro-nit: The commit message of 5/6 starts \"As we'll see in a\nsubsequent commit..\", which may need a slight tweak if 6/6 becomes 'far\naway' in the commit tree.\n\n--\nPhilip\n"},{"id":"470452","messageId":"xmqqlem2coix.fsf@gitster.g","threadId":"59062","inReplyTo":"b2eca967-4c52-3f8d-f3fe-b1ed41b06280@iee.email","subject":"Re: [PATCH v2 0/6] cache API: always have a \"istate->repo\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-16T18:39:18Z","receivedAt":"2023-01-16T18:44:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> On 16/01/2023 13:38, Ævar Arnfjörð Bjarmason wrote:\n>> On Fri, Jan 13 2023, Junio C Hamano wrote:\n>>\n>>> Ævar Arnfjörð Bjarmason writes:\n>>>\n>>>> The \"struct index_state\" contains a \"repo\" member, which should be a\n>>>> pointer to the repository for that index, but which due to us\n>>>> constructing such structs on an ad-hoc basis in various places wasn't\n>>>> always available.\n>>> I'd exclude 6/6 for now, as it seems to depend on some changes only\n>>> in 'next'. Feel free to resend only that step with reduced scope so\n>>> that it applies to 'master', and send in incremental updates when\n>>> each of these topics that are only in 'next' graduates.\n>> Okey, the 6/6 requires ds/omit-trailing-hash-in-index. As both it and\n>> the 1-5/6 of this are in \"next\" now I think it's best that I just submit\n>> the 6/6 stand-alone after both of those have graduated.\n>>\n> micro-nit: The commit message of 5/6 starts \"As we'll see in a\n> subsequent commit..\", which may need a slight tweak if 6/6 becomes 'far\n> away' in the commit tree.\n\nIndeed. \n\nI'd use \n\n\tHopefully in some not so distant future, we'll get\n\tadvantages from always initializing the \"repo\" member ...\n\nfor now.\n"},{"id":"470492","messageId":"patch-1.1-b4998652822-20230117T135234Z-avarab@gmail.com","threadId":"59062","inReplyTo":"xmqqlem2coix.fsf@gitster.g","subject":"[PATCH] treewide: always have a valid \"index_state.repo\" member","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-17T13:57:00Z","receivedAt":"2023-01-17T13:58:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the \"repo\" member was added to \"the_index\" in [1] the\nrepo_read_index() was made to populate it, but the unpopulated\n\"the_index\" variable didn't get the same treatment.\n\nLet's do that in initialize_the_repository() when we set it up, and\nlikewise for all of the current callers initialized an empty \"struct\nindex_state\".\n\nThis simplifies code that needs to deal with \"the_index\" or a custom\n\"struct index_state\", we no longer need to second-guess this part of\nthe \"index_state\" deep in the stack. A recent example of such\nsecond-guessing is the \"istate->repo ? istate->repo : the_repository\"\ncode in [2]. We can now simply use \"istate->repo\".\n\nWe're doing this by making use of the INDEX_STATE_INIT() macro (and\ncorresponding function) added in [3], which now have mandatory \"repo\"\narguments.\n\nBecause we now call index_state_init() in repository.c's\ninitialize_the_repository() we don't need to handle the case where we\nhave a \"repo->index\" whose \"repo\" member doesn't match the \"repo\"\nwe're setting up, i.e. the \"Complete the double-reference\" code in\nrepo_read_index() being altered here. That logic was originally added\nin [1], and was working around the lack of what we now have in\ninitialize_the_repository().\n\nFor \"fsmonitor-settings.c\" we can remove the initialization of a NULL\n\"r\" argument to \"the_repository\". This was added back in [4], and was\nneeded at the time for callers that would pass us the \"r\" from an\n\"istate->repo\". Before this change such a change to\n\"fsmonitor-settings.c\" would segfault all over the test suite (e.g. in\nt0002-gitfile.sh).\n\nThis change has wider eventual implications for\n\"fsmonitor-settings.c\". The reason the other lazy loading behavior in\nit is required (starting with \"if (!r->settings.fsmonitor) ...\" is\nbecause of the previously passed \"r\" being \"NULL\".\n\nI have other local changes on top of this which move its configuration\nreading to \"prepare_repo_settings()\" in \"repo-settings.c\", as we could\nnow start to rely on it being called for our \"r\". But let's leave all\nof that for now, and narrowly remove this particular part of the\nlazy-loading.\n\n1. 1fd9ae517c4 (repository: add repo reference to index_state,\n 2021-01-23)\n2. ee1f0c242ef (read-cache: add index.skipHash config option,\n 2023-01-06)\n3. 2f6b1eb794e (cache API: add a \"INDEX_STATE_INIT\" macro/function,\n add release_index(), 2023-01-12)\n4. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,\n 2022-03-25)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nThis an updated verison of the 6/6 of [A], which per Junio's [B]\nwasn't picked up with those patches, which are now in \"next\".\n\nJunio: Now that Derrick's ds/omit-trailing-hash-in-index has landed on\n\"master\" this can be applied on top a merge of \"master\" and what you\nhave in \"ab/cache-api-cleanup\" (that topic itself being based on a\ntoo-old \"master\").\n\nSince the v2 I changed the \"Complete the double-reference\" logic in\nrepo_read_index() so that we're not working around a state of a\naffairs that no longer exists with this change.\n\nA. https://lore.kernel.org/git/cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com/\nB. https://lore.kernel.org/git/xmqqtu0u2q9u.fsf@gitster.g/\n\nRange-diff:\n1: 291233b0420 ! 1: b4998652822 treewide: always have a valid \"index_state.repo\" member\n @@ Commit message\n code in [2]. We can now simply use \"istate->repo\".\n \n We're doing this by making use of the INDEX_STATE_INIT() macro (and\n - corresponding function) added in the preceding commit, which now have\n - mandatory \"repo\" arguments. As seen here there are cases where we set\n - it to NULL, but only if we're about to fill in the correct non-NULL\n - \"repo\" right afterwards.\n + corresponding function) added in [3], which now have mandatory \"repo\"\n + arguments.\n +\n + Because we now call index_state_init() in repository.c's\n + initialize_the_repository() we don't need to handle the case where we\n + have a \"repo->index\" whose \"repo\" member doesn't match the \"repo\"\n + we're setting up, i.e. the \"Complete the double-reference\" code in\n + repo_read_index() being altered here. That logic was originally added\n + in [1], and was working around the lack of what we now have in\n + initialize_the_repository().\n \n For \"fsmonitor-settings.c\" we can remove the initialization of a NULL\n - \"r\" argument to \"the_repository\". This was added back in [3], and was\n + \"r\" argument to \"the_repository\". This was added back in [4], and was\n needed at the time for callers that would pass us the \"r\" from an\n \"istate->repo\". Before this change such a change to\n \"fsmonitor-settings.c\" would segfault all over the test suite (e.g. in\n @@ Commit message\n 2021-01-23)\n 2. ee1f0c242ef (read-cache: add index.skipHash config option,\n 2023-01-06)\n - 3. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,\n + 3. 2f6b1eb794e (cache API: add a \"INDEX_STATE_INIT\" macro/function,\n + add release_index(), 2023-01-12)\n + 4. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,\n 2022-03-25)\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n @@ repository.c: void initialize_the_repository(void)\n }\n \n @@ repository.c: int repo_read_index(struct repository *repo)\n + {\n + \tint res;\n \n ++\t/* Complete the double-reference */\n \tif (!repo->index) {\n \t\tALLOC_ARRAY(repo->index, 1);\n -\t\tindex_state_init(repo->index);\n -+\t\tindex_state_init(repo->index, NULL /* set below */);\n - \t}\n +-\t}\n +-\n +-\t/* Complete the double-reference */\n +-\tif (!repo->index->repo)\n +-\t\trepo->index->repo = repo;\n +-\telse if (repo->index->repo != repo)\n ++\t\tindex_state_init(repo->index, repo);\n ++\t} else if (repo->index->repo != repo) {\n + \t\tBUG(\"repo's index should point back at itself\");\n ++\t}\n + \n + \tres = read_index_from(repo->index, repo->index_file, repo->gitdir);\n \n - \t/* Complete the double-reference */\n \n ## revision.c ##\n @@ revision.c: void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n\n apply.c | 2 +-\n builtin/difftool.c | 2 +-\n builtin/sparse-checkout.c | 2 +-\n builtin/stash.c | 8 ++++----\n builtin/worktree.c | 2 +-\n cache.h | 9 ++++++---\n fsmonitor-settings.c | 14 --------------\n fsmonitor.c | 2 +-\n merge-recursive.c | 2 +-\n read-cache.c | 17 +++++------------\n repository.c | 13 ++++++-------\n revision.c | 2 +-\n sparse-index.c | 9 ---------\n split-index.c | 2 +-\n unpack-trees.c | 2 +-\n 15 files changed, 30 insertions(+), 58 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 821fe411ec8..5eec433583e 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4105,7 +4105,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p, struct object_id *oid)\n static int build_fake_ancestor(struct apply_state *state, struct patch *list)\n {\n \tstruct patch *patch;\n-\tstruct index_state result = INDEX_STATE_INIT;\n+\tstruct index_state result = INDEX_STATE_INIT(state->repo);\n \tstruct lock_file lock = LOCK_INIT;\n \tint res;\n \ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex 758e7bd3b6a..dbbfb19f192 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \tstruct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL);\n \tstruct hashmap_iter iter;\n \tstruct pair_entry *entry;\n-\tstruct index_state wtindex = INDEX_STATE_INIT;\n+\tstruct index_state wtindex = INDEX_STATE_INIT(the_repository);\n \tstruct checkout lstate, rstate;\n \tint err = 0;\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex dc332c6d05f..c3738154918 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -217,7 +217,7 @@ static int update_working_directory(struct pattern_list *pl)\n \to.head_idx = -1;\n \to.src_index = r->index;\n \to.dst_index = r->index;\n-\tindex_state_init(&o.result);\n+\tindex_state_init(&o.result, r);\n \to.skip_sparse_checkout = 0;\n \to.pl = pl;\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex a5f13fb1e9f..839569a9803 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1137,7 +1137,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg,\n \tint ret = 0;\n \tstruct strbuf untracked_msg = STRBUF_INIT;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tcp_upd_index.git_cmd = 1;\n \tstrvec_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n@@ -1176,7 +1176,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch,\n {\n \tint ret = 0;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tif (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,\n \t\t\t\t0, NULL)) {\n@@ -1209,7 +1209,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \tint ret = 0;\n \tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n@@ -1271,7 +1271,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \tstruct rev_info rev;\n \tstruct child_process cp_upd_index = CHILD_PROCESS_INIT;\n \tstruct strbuf diff_output = STRBUF_INIT;\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \n \tinit_revisions(&rev, NULL);\n \tcopy_pathspec(&rev.prune_data, ps);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 311d6e90750..f51c40f1e1e 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -923,7 +923,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)\n \n static void validate_no_submodules(const struct worktree *wt)\n {\n-\tstruct index_state istate = INDEX_STATE_INIT;\n+\tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tstruct strbuf path = STRBUF_INIT;\n \tint i, found_submodules = 0;\n \ndiff --git a/cache.h b/cache.h\nindex be28fce12ac..4bf14e0bd94 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -367,10 +367,13 @@ struct index_state {\n * If the variable won't be used again, use release_index() to free()\n * its resources. If it needs to be used again use discard_index(),\n * which does the same thing, but will use use index_state_init() at\n- * the end.\n+ * the end. The discard_index() will use its own \"istate->repo\" as the\n+ * \"r\" argument to index_state_init() in that case.\n */\n-#define INDEX_STATE_INIT { 0 }\n-void index_state_init(struct index_state *istate);\n+#define INDEX_STATE_INIT(r) { \\\n+\t.repo = (r), \\\n+}\n+void index_state_init(struct index_state *istate, struct repository *r);\n void release_index(struct index_state *istate);\n \n /* Name hashing */\ndiff --git a/fsmonitor-settings.c b/fsmonitor-settings.c\nindex ee63a97dc51..899bfe9c813 100644\n--- a/fsmonitor-settings.c\n+++ b/fsmonitor-settings.c\n@@ -143,8 +143,6 @@ static void lookup_fsmonitor_settings(struct repository *r)\n \n enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \n@@ -153,8 +151,6 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)\n \n const char *fsm_settings__get_hook_path(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \n@@ -174,8 +170,6 @@ void fsm_settings__set_ipc(struct repository *r)\n \t * Caller requested IPC explicitly, so avoid (possibly\n \t * recursive) config lookup.\n \t */\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -197,8 +191,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)\n \t * Caller requested hook explicitly, so avoid (possibly\n \t * recursive) config lookup.\n \t */\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -210,8 +202,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)\n \n void fsm_settings__set_disabled(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -223,8 +213,6 @@ void fsm_settings__set_disabled(struct repository *r)\n void fsm_settings__set_incompatible(struct repository *r,\n \t\t\t\t enum fsmonitor_reason reason)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tr->settings.fsmonitor = alloc_settings();\n \n@@ -235,8 +223,6 @@ void fsm_settings__set_incompatible(struct repository *r,\n \n enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)\n {\n-\tif (!r)\n-\t\tr = the_repository;\n \tif (!r->settings.fsmonitor)\n \t\tlookup_fsmonitor_settings(r);\n \ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 08af00c7387..a5b9e75437b 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -304,7 +304,7 @@ void refresh_fsmonitor(struct index_state *istate)\n \tchar *buf;\n \tunsigned int i;\n \tint is_trivial = 0;\n-\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n+\tstruct repository *r = istate->repo;\n \tenum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);\n \tenum fsmonitor_reason reason = fsm_settings__get_reason(r);\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 0948b3ea961..ae469f8cc81 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -412,7 +412,7 @@ static int unpack_trees_start(struct merge_options *opt,\n {\n \tint rc;\n \tstruct tree_desc t[3];\n-\tstruct index_state tmp_index = INDEX_STATE_INIT;\n+\tstruct index_state tmp_index = INDEX_STATE_INIT(opt->repo);\n \n \tmemset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));\n \tif (opt->priv->call_depth)\ndiff --git a/read-cache.c b/read-cache.c\nindex d191741f600..7bd12afb38c 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2300,8 +2300,6 @@ static void set_new_index_sparsity(struct index_state *istate)\n \t * If the index's repo exists, mark it sparse according to\n \t * repo settings.\n \t */\n-\tif (!istate->repo)\n-\t\treturn;\n \tprepare_repo_settings(istate->repo);\n \tif (!istate->repo->settings.command_requires_full_index &&\n \t is_sparse_index_allowed(istate, 0))\n@@ -2330,8 +2328,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tfd = open(path, O_RDONLY);\n \tif (fd < 0) {\n \t\tif (!must_exist && errno == ENOENT) {\n-\t\t\tif (!istate->repo)\n-\t\t\t\tistate->repo = the_repository;\n \t\t\tset_new_index_sparsity(istate);\n \t\t\treturn 0;\n \t\t}\n@@ -2433,9 +2429,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \ttrace2_data_intmax(\"index\", the_repository, \"read/cache_nr\",\n \t\t\t istate->cache_nr);\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \t/*\n \t * If the command explicitly requires a full index, force it\n \t * to be full. Otherwise, correct the sparsity based on repository\n@@ -2501,7 +2494,7 @@ int read_index_from(struct index_state *istate, const char *path,\n \t\trelease_index(split_index->base);\n \telse\n \t\tALLOC_ARRAY(split_index->base, 1);\n-\tindex_state_init(split_index->base);\n+\tindex_state_init(split_index->base, istate->repo);\n \n \tbase_oid_hex = oid_to_hex(&split_index->base_oid);\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n@@ -2540,9 +2533,9 @@ int is_index_unborn(struct index_state *istate)\n \treturn (!istate->cache_nr && !istate->timestamp.sec);\n }\n \n-void index_state_init(struct index_state *istate)\n+void index_state_init(struct index_state *istate, struct repository *r)\n {\n-\tstruct index_state blank = INDEX_STATE_INIT;\n+\tstruct index_state blank = INDEX_STATE_INIT(r);\n \tmemcpy(istate, &blank, sizeof(*istate));\n }\n \n@@ -2579,7 +2572,7 @@ void release_index(struct index_state *istate)\n void discard_index(struct index_state *istate)\n {\n \trelease_index(istate);\n-\tindex_state_init(istate);\n+\tindex_state_init(istate, istate->repo);\n }\n \n /*\n@@ -2933,7 +2926,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tint ieot_entries = 1;\n \tstruct index_entry_offset_table *ieot = NULL;\n \tint nr, nr_threads;\n-\tstruct repository *r = istate->repo ? istate->repo : the_repository;\n+\tstruct repository *r = istate->repo;\n \n \tf = hashfd(tempfile->fd, tempfile->filename.buf);\n \ndiff --git a/repository.c b/repository.c\nindex a5805fa33b1..937fa974b38 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -28,6 +28,8 @@ void initialize_the_repository(void)\n \tthe_repo.remote_state = remote_state_new();\n \tthe_repo.parsed_objects = parsed_object_pool_new();\n \n+\tindex_state_init(&the_index, the_repository);\n+\n \trepo_set_hash_algo(&the_repo, GIT_HASH_SHA1);\n }\n \n@@ -302,16 +304,13 @@ int repo_read_index(struct repository *repo)\n {\n \tint res;\n \n+\t/* Complete the double-reference */\n \tif (!repo->index) {\n \t\tALLOC_ARRAY(repo->index, 1);\n-\t\tindex_state_init(repo->index);\n-\t}\n-\n-\t/* Complete the double-reference */\n-\tif (!repo->index->repo)\n-\t\trepo->index->repo = repo;\n-\telse if (repo->index->repo != repo)\n+\t\tindex_state_init(repo->index, repo);\n+\t} else if (repo->index->repo != repo) {\n \t\tBUG(\"repo's index should point back at itself\");\n+\t}\n \n \tres = read_index_from(repo->index, repo->index_file, repo->gitdir);\n \ndiff --git a/revision.c b/revision.c\nindex fb090886f5b..21f5f572c22 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1813,7 +1813,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \tworktrees = get_worktrees();\n \tfor (p = worktrees; *p; p++) {\n \t\tstruct worktree *wt = *p;\n-\t\tstruct index_state istate = INDEX_STATE_INIT;\n+\t\tstruct index_state istate = INDEX_STATE_INIT(revs->repo);\n \n \t\tif (wt->is_current)\n \t\t\tcontinue; /* current index already taken care of */\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 86e3b99870b..147a13386a4 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -128,9 +128,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)\n \tif (!core_apply_sparse_checkout || !core_sparse_checkout_cone)\n \t\treturn 0;\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \tif (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {\n \t\tint test_env;\n \n@@ -327,9 +324,6 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \t\t\tpl = NULL;\n \t}\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \t/*\n \t * A NULL pattern set indicates we are expanding a full index, so\n \t * we use a special region name that indicates the full expansion.\n@@ -552,9 +546,6 @@ void expand_to_path(struct index_state *istate,\n \tif (!istate->sparse_index)\n \t\treturn;\n \n-\tif (!istate->repo)\n-\t\tistate->repo = the_repository;\n-\n \tin_expand_to_path = 1;\n \n \t/*\ndiff --git a/split-index.c b/split-index.c\nindex a5b56c0c37f..5d0f04763ea 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -91,7 +91,7 @@ void move_cache_to_base_index(struct index_state *istate)\n \t}\n \n \tALLOC_ARRAY(si->base, 1);\n-\tindex_state_init(si->base);\n+\tindex_state_init(si->base, istate->repo);\n \tsi->base->version = istate->version;\n \t/* zero timestamp disables racy test in ce_write_index() */\n \tsi->base->timestamp = istate->timestamp;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex d1d1b0f319e..3d05e45a279 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1905,7 +1905,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\tpopulate_from_existing_patterns(o, &pl);\n \t}\n \n-\tindex_state_init(&o->result);\n+\tindex_state_init(&o->result, o->src_index->repo);\n \to->result.initialized = 1;\n \to->result.timestamp.sec = o->src_index->timestamp.sec;\n \to->result.timestamp.nsec = o->src_index->timestamp.nsec;\n-- \n2.39.0.1225.g30a3d88132d\n\n"},{"id":"470501","messageId":"db312853-81a1-542b-db96-d816c463516c@github.com","threadId":"59062","inReplyTo":"patch-1.1-b4998652822-20230117T135234Z-avarab@gmail.com","subject":"Re: [PATCH] treewide: always have a valid \"index_state.repo\" member","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-17T15:34:11Z","receivedAt":"2023-01-17T15:34:16Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/17/2023 8:57 AM, Ævar Arnfjörð Bjarmason wrote:\n \n> This an updated verison of the 6/6 of [A], which per Junio's [B]\n> wasn't picked up with those patches, which are now in \"next\".\n> \n> Junio: Now that Derrick's ds/omit-trailing-hash-in-index has landed on\n> \"master\" this can be applied on top a merge of \"master\" and what you\n> have in \"ab/cache-api-cleanup\" (that topic itself being based on a\n> too-old \"master\").\n> \n> Since the v2 I changed the \"Complete the double-reference\" logic in\n> repo_read_index() so that we're not working around a state of a\n> affairs that no longer exists with this change.\n> \n> A. https://lore.kernel.org/git/cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com/\n> B. https://lore.kernel.org/git/xmqqtu0u2q9u.fsf@gitster.g/\n> \n> Range-diff:\n\nThese changes look good to me. Thanks for dealing with the branch-\nhopping.\n\nThanks,\n-Stolee\n\n"}]}