From 0dda3ac92564e4e3f26a341103c67629e926ab40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 13:55:23 +0100 Subject: [PATCH 1/5] builtin/difftool.c: { 0 }-initialize rather than using memset() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor an initialization of a variable added in 03831ef7b50 (difftool: implement the functionality in the builtin, 2017-01-19). This refactoring makes a subsequent change smaller. Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/difftool.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index d9b76226f6..1f9d4324df 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, struct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL); struct hashmap_iter iter; struct pair_entry *entry; - struct index_state wtindex; + struct index_state wtindex = { 0 }; struct checkout lstate, rstate; int err = 0; struct child_process cmd = CHILD_PROCESS_INIT; @@ -387,8 +387,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, mkdir(ldir.buf, 0700); mkdir(rdir.buf, 0700); - memset(&wtindex, 0, sizeof(wtindex)); - memset(&lstate, 0, sizeof(lstate)); lstate.base_dir = lbase_dir = xstrdup(ldir.buf); lstate.base_dir_len = ldir.len; From d2cdf2c285b091f44e0bdc154ee39c0071f8934e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 13:55:24 +0100 Subject: [PATCH 2/5] sparse-index.c: expand_to_path() can assume non-NULL "istate" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function added in [1] was subsequently used in [2]. All of the calls to it are in name-hash.c, and come after calls to lazy_init_name_hash(istate). The first thing that function does is: if (istate->name_hash_initialized) return; So we can already assume that we have a non-NULL "istate" here, or we'd be segfaulting. Let's not confuse matters by making it appear that's not the case. 1. 71f82d032f3 (sparse-index: expand_to_path(), 2021-04-12) 2. 4589bca829a (name-hash: use expand_to_path(), 2021-04-12) Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- sparse-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sparse-index.c b/sparse-index.c index 8c269dab80..65a08d33c7 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -547,7 +547,7 @@ void expand_to_path(struct index_state *istate, if (in_expand_to_path) return; - if (!istate || !istate->sparse_index) + if (!istate->sparse_index) return; if (!istate->repo) From 29fefafcba0a3608465491835d84b646ce1e1b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 13:55:25 +0100 Subject: [PATCH 3/5] sparse-index API: BUG() out on NULL ensure_full_index() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the ensure_full_index() function stricter, and have it only accept a non-NULL "struct index_state". This function (and this behavior) was added in [1]. The only reason it needed to be this lax was due to interaction with repo_index_has_changes(). See the addition of that code in [2]. The other reason for why this was needed dates back to interaction with code added in [3]. In [4] we started calling ensure_full_index() in unpack_trees(), but the caller added in 34110cd4e39 wants to pass us a NULL "dst_index". Let's instead do the NULL check in unpack_trees() itself. 1. 4300f8442a2 (sparse-index: implement ensure_full_index(), 2021-03-30) 2. 0c18c059a15 (read-cache: ensure full index, 2021-04-01) 3. 34110cd4e39 (Make 'unpack_trees()' have a separate source and destination index, 2008-03-06) 4. 6863df35503 (unpack-trees: ensure full index, 2021-03-30) Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- sparse-index.c | 4 +++- unpack-trees.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/sparse-index.c b/sparse-index.c index 65a08d33c7..86e3b99870 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -299,7 +299,7 @@ void expand_index(struct index_state *istate, struct pattern_list *pl) * If the index is already full, then keep it full. We will convert * it to a sparse index on write, if possible. */ - if (!istate || istate->sparse_index == INDEX_EXPANDED) + if (istate->sparse_index == INDEX_EXPANDED) return; /* @@ -424,6 +424,8 @@ void expand_index(struct index_state *istate, struct pattern_list *pl) void ensure_full_index(struct index_state *istate) { + if (!istate) + BUG("ensure_full_index() must get an index!"); expand_index(istate, NULL); } diff --git a/unpack-trees.c b/unpack-trees.c index ea09023b01..2381cd7cac 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1880,7 +1880,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options prepare_repo_settings(repo); if (repo->settings.command_requires_full_index) { ensure_full_index(o->src_index); - ensure_full_index(o->dst_index); + if (o->dst_index) + ensure_full_index(o->dst_index); } if (o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED && From 5bdf6d4ac0d06817ce22322aa0172b49e6026544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 13:55:26 +0100 Subject: [PATCH 4/5] read-cache.c: refactor set_new_index_sparsity() for subsequent commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor code added to set_new_index_sparsity() in [1] to eliminate indentation resulting from putting the body of his function within the "if" block. Let's instead return early if we have no istate->repo. This trivial change makes the subsequent commit's diff smaller. 1. 491df5f679f (read-cache: set sparsity when index is new, 2022-05-10) Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- read-cache.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/read-cache.c b/read-cache.c index 1ff518b2a7..1782e94035 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2292,12 +2292,12 @@ static void set_new_index_sparsity(struct index_state *istate) * If the index's repo exists, mark it sparse according to * repo settings. */ - if (istate->repo) { - prepare_repo_settings(istate->repo); - if (!istate->repo->settings.command_requires_full_index && - is_sparse_index_allowed(istate, 0)) - istate->sparse_index = 1; - } + if (!istate->repo) + return; + prepare_repo_settings(istate->repo); + if (!istate->repo->settings.command_requires_full_index && + is_sparse_index_allowed(istate, 0)) + istate->sparse_index = 1; } /* remember to discard_cache() before reading a different cache! */ From 2f6b1eb794ee1f152c1a4b519e3b6dcecd0b487f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 13:55:27 +0100 Subject: [PATCH 5/5] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hopefully in some not so distant future, we'll get advantages from always initializing the "repo" member of the "struct index_state". To make that easier let's introduce an initialization macro & function. The various ad-hoc initialization of the structure can then be changed over to it, and we can remove the various "0" assignments in discard_index() in favor of calling index_state_init() at the end. While not strictly necessary, let's also change the CALLOC_ARRAY() of various "struct index_state *" to use an ALLOC_ARRAY() followed by index_state_init() instead. We're then adding the release_index() function and converting some callers (including some of these allocations) over to it if they either won't need to use their "struct index_state" again, or are just about to call index_state_init(). Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- apply.c | 2 +- builtin/difftool.c | 2 +- builtin/sparse-checkout.c | 1 + builtin/stash.c | 16 ++++++++-------- builtin/worktree.c | 2 +- cache.h | 13 +++++++++++++ merge-recursive.c | 2 +- read-cache.c | 31 ++++++++++++++++++------------- repository.c | 6 ++++-- revision.c | 2 +- split-index.c | 3 ++- unpack-trees.c | 2 +- 12 files changed, 52 insertions(+), 30 deletions(-) diff --git a/apply.c b/apply.c index 8582228047..821fe411ec 100644 --- a/apply.c +++ b/apply.c @@ -4105,7 +4105,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p, struct object_id *oid) static int build_fake_ancestor(struct apply_state *state, struct patch *list) { struct patch *patch; - struct index_state result = { NULL }; + struct index_state result = INDEX_STATE_INIT; struct lock_file lock = LOCK_INIT; int res; diff --git a/builtin/difftool.c b/builtin/difftool.c index 1f9d4324df..758e7bd3b6 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -361,7 +361,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, struct hashmap symlinks2 = HASHMAP_INIT(pair_cmp, NULL); struct hashmap_iter iter; struct pair_entry *entry; - struct index_state wtindex = { 0 }; + struct index_state wtindex = INDEX_STATE_INIT; struct checkout lstate, rstate; int err = 0; struct child_process cmd = CHILD_PROCESS_INIT; diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 58a22503f0..dc332c6d05 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -217,6 +217,7 @@ static int update_working_directory(struct pattern_list *pl) o.head_idx = -1; o.src_index = r->index; o.dst_index = r->index; + index_state_init(&o.result); o.skip_sparse_checkout = 0; o.pl = pl; diff --git a/builtin/stash.c b/builtin/stash.c index bb0fd86143..a5f13fb1e9 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1137,7 +1137,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg, int ret = 0; struct strbuf untracked_msg = STRBUF_INIT; struct child_process cp_upd_index = CHILD_PROCESS_INIT; - struct index_state istate = { NULL }; + struct index_state istate = INDEX_STATE_INIT; cp_upd_index.git_cmd = 1; strvec_pushl(&cp_upd_index.args, "update-index", "-z", "--add", @@ -1165,7 +1165,7 @@ static int save_untracked_files(struct stash_info *info, struct strbuf *msg, } done: - discard_index(&istate); + release_index(&istate); strbuf_release(&untracked_msg); remove_path(stash_index_path.buf); return ret; @@ -1176,7 +1176,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch, { int ret = 0; struct child_process cp_diff_tree = CHILD_PROCESS_INIT; - struct index_state istate = { NULL }; + struct index_state istate = INDEX_STATE_INIT; if (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file, 0, NULL)) { @@ -1199,7 +1199,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch, } done: - discard_index(&istate); + release_index(&istate); return ret; } @@ -1209,7 +1209,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, int ret = 0; struct child_process cp_read_tree = CHILD_PROCESS_INIT; struct child_process cp_diff_tree = CHILD_PROCESS_INIT; - struct index_state istate = { NULL }; + struct index_state istate = INDEX_STATE_INIT; char *old_index_env = NULL, *old_repo_index_file; remove_path(stash_index_path.buf); @@ -1260,7 +1260,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, } done: - discard_index(&istate); + release_index(&istate); remove_path(stash_index_path.buf); return ret; } @@ -1271,7 +1271,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps struct rev_info rev; struct child_process cp_upd_index = CHILD_PROCESS_INIT; struct strbuf diff_output = STRBUF_INIT; - struct index_state istate = { NULL }; + struct index_state istate = INDEX_STATE_INIT; init_revisions(&rev, NULL); copy_pathspec(&rev.prune_data, ps); @@ -1319,7 +1319,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps } done: - discard_index(&istate); + release_index(&istate); release_revisions(&rev); strbuf_release(&diff_output); remove_path(stash_index_path.buf); diff --git a/builtin/worktree.c b/builtin/worktree.c index 591d659fae..311d6e9075 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -923,7 +923,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix) static void validate_no_submodules(const struct worktree *wt) { - struct index_state istate = { NULL }; + struct index_state istate = INDEX_STATE_INIT; struct strbuf path = STRBUF_INIT; int i, found_submodules = 0; diff --git a/cache.h b/cache.h index 21ed9633b2..be28fce12a 100644 --- a/cache.h +++ b/cache.h @@ -360,6 +360,19 @@ struct index_state { struct pattern_list *sparse_checkout_patterns; }; +/** + * A "struct index_state istate" must be initialized with + * INDEX_STATE_INIT or the corresponding index_state_init(). + * + * If the variable won't be used again, use release_index() to free() + * its resources. If it needs to be used again use discard_index(), + * which does the same thing, but will use use index_state_init() at + * the end. + */ +#define INDEX_STATE_INIT { 0 } +void index_state_init(struct index_state *istate); +void release_index(struct index_state *istate); + /* Name hashing */ int test_lazy_init_name_hash(struct index_state *istate, int try_threaded); void add_name_hash(struct index_state *istate, struct cache_entry *ce); diff --git a/merge-recursive.c b/merge-recursive.c index 2fd0aa9687..0948b3ea96 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -412,7 +412,7 @@ static int unpack_trees_start(struct merge_options *opt, { int rc; struct tree_desc t[3]; - struct index_state tmp_index = { NULL }; + struct index_state tmp_index = INDEX_STATE_INIT; memset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts)); if (opt->priv->call_depth) diff --git a/read-cache.c b/read-cache.c index 1782e94035..a91ec90f59 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2490,9 +2490,10 @@ int read_index_from(struct index_state *istate, const char *path, trace_performance_enter(); if (split_index->base) - discard_index(split_index->base); + release_index(split_index->base); else - CALLOC_ARRAY(split_index->base, 1); + ALLOC_ARRAY(split_index->base, 1); + index_state_init(split_index->base); base_oid_hex = oid_to_hex(&split_index->base_oid); base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex); @@ -2531,7 +2532,13 @@ int is_index_unborn(struct index_state *istate) return (!istate->cache_nr && !istate->timestamp.sec); } -void discard_index(struct index_state *istate) +void index_state_init(struct index_state *istate) +{ + struct index_state blank = INDEX_STATE_INIT; + memcpy(istate, &blank, sizeof(*istate)); +} + +void release_index(struct index_state *istate) { /* * Cache entries in istate->cache[] should have been allocated @@ -2543,20 +2550,12 @@ void discard_index(struct index_state *istate) validate_cache_entries(istate); resolve_undo_clear_index(istate); - istate->cache_nr = 0; - istate->cache_changed = 0; - istate->timestamp.sec = 0; - istate->timestamp.nsec = 0; free_name_hash(istate); cache_tree_free(&(istate->cache_tree)); - istate->initialized = 0; - istate->fsmonitor_has_run_once = 0; - FREE_AND_NULL(istate->fsmonitor_last_update); - FREE_AND_NULL(istate->cache); - istate->cache_alloc = 0; + free(istate->fsmonitor_last_update); + free(istate->cache); discard_split_index(istate); free_untracked_cache(istate->untracked); - istate->untracked = NULL; if (istate->sparse_checkout_patterns) { clear_pattern_list(istate->sparse_checkout_patterns); @@ -2569,6 +2568,12 @@ void discard_index(struct index_state *istate) } } +void discard_index(struct index_state *istate) +{ + release_index(istate); + index_state_init(istate); +} + /* * Validate the cache entries of this index. * All cache entries associated with this index diff --git a/repository.c b/repository.c index 3427085fd6..a5805fa33b 100644 --- a/repository.c +++ b/repository.c @@ -302,8 +302,10 @@ int repo_read_index(struct repository *repo) { int res; - if (!repo->index) - CALLOC_ARRAY(repo->index, 1); + if (!repo->index) { + ALLOC_ARRAY(repo->index, 1); + index_state_init(repo->index); + } /* Complete the double-reference */ if (!repo->index->repo) diff --git a/revision.c b/revision.c index 100e5ad511..fb090886f5 100644 --- a/revision.c +++ b/revision.c @@ -1813,7 +1813,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags) worktrees = get_worktrees(); for (p = worktrees; *p; p++) { struct worktree *wt = *p; - struct index_state istate = { NULL }; + struct index_state istate = INDEX_STATE_INIT; if (wt->is_current) continue; /* current index already taken care of */ diff --git a/split-index.c b/split-index.c index 9d0ccc30d0..a5b56c0c37 100644 --- a/split-index.c +++ b/split-index.c @@ -90,7 +90,8 @@ void move_cache_to_base_index(struct index_state *istate) mem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool); } - CALLOC_ARRAY(si->base, 1); + ALLOC_ARRAY(si->base, 1); + index_state_init(si->base); si->base->version = istate->version; /* zero timestamp disables racy test in ce_write_index() */ si->base->timestamp = istate->timestamp; diff --git a/unpack-trees.c b/unpack-trees.c index 2381cd7cac..d1d1b0f319 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1905,7 +1905,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options populate_from_existing_patterns(o, &pl); } - memset(&o->result, 0, sizeof(o->result)); + index_state_init(&o->result); o->result.initialized = 1; o->result.timestamp.sec = o->src_index->timestamp.sec; o->result.timestamp.nsec = o->src_index->timestamp.nsec;