* [PATCH v2 1/6] builtin/difftool.c: { 0 }-initialize rather than using memset()
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
@ 2023-01-12 12:55 ` Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 2/6] sparse-index.c: expand_to_path() can assume non-NULL "istate" Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 12:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
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 <avarab@gmail.com>
---
builtin/difftool.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index d9b76226f6a..1f9d4324df5 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;
--
2.39.0.1205.g2ca064edc27
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 2/6] sparse-index.c: expand_to_path() can assume non-NULL "istate"
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 1/6] builtin/difftool.c: { 0 }-initialize rather than using memset() Ævar Arnfjörð Bjarmason
@ 2023-01-12 12:55 ` Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 3/6] sparse-index API: BUG() out on NULL ensure_full_index() Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 12:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
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 <avarab@gmail.com>
---
sparse-index.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sparse-index.c b/sparse-index.c
index 8c269dab803..65a08d33c73 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)
--
2.39.0.1205.g2ca064edc27
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 3/6] sparse-index API: BUG() out on NULL ensure_full_index()
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 1/6] builtin/difftool.c: { 0 }-initialize rather than using memset() Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 2/6] sparse-index.c: expand_to_path() can assume non-NULL "istate" Ævar Arnfjörð Bjarmason
@ 2023-01-12 12:55 ` Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 4/6] read-cache.c: refactor set_new_index_sparsity() for subsequent commit Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 12:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
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 <avarab@gmail.com>
---
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 65a08d33c73..86e3b99870b 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 ea09023b015..2381cd7cac4 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 &&
--
2.39.0.1205.g2ca064edc27
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 4/6] read-cache.c: refactor set_new_index_sparsity() for subsequent commit
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2023-01-12 12:55 ` [PATCH v2 3/6] sparse-index API: BUG() out on NULL ensure_full_index() Ævar Arnfjörð Bjarmason
@ 2023-01-12 12:55 ` Ævar Arnfjörð Bjarmason
2023-01-12 12:55 ` [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index() Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 12:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
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 <avarab@gmail.com>
---
read-cache.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index cf87ad70970..25925c8c002 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2300,12 +2300,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! */
--
2.39.0.1205.g2ca064edc27
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2023-01-12 12:55 ` [PATCH v2 4/6] read-cache.c: refactor set_new_index_sparsity() for subsequent commit Ævar Arnfjörð Bjarmason
@ 2023-01-12 12:55 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:21 ` Derrick Stolee
2023-01-12 12:55 ` [PATCH v2 6/6] treewide: always have a valid "index_state.repo" member Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 12:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
As we'll see in a subsequent commit 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 <avarab@gmail.com>
---
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 85822280476..821fe411ec8 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 1f9d4324df5..758e7bd3b6a 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 58a22503f04..dc332c6d05f 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 bb0fd861434..a5f13fb1e9f 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 591d659faea..311d6e90750 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 21ed9633b2a..be28fce12ac 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 2fd0aa96875..0948b3ea961 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 25925c8c002..d191741f600 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2498,9 +2498,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);
@@ -2539,7 +2540,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
@@ -2551,20 +2558,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);
@@ -2577,6 +2576,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 3427085fd6d..a5805fa33b1 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 100e5ad5110..fb090886f5b 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 9d0ccc30d00..a5b56c0c37f 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 2381cd7cac4..d1d1b0f319e 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;
--
2.39.0.1205.g2ca064edc27
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()
2023-01-12 12:55 ` [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index() Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:21 ` Derrick Stolee
2023-01-12 16:19 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 32+ messages in thread
From: Derrick Stolee @ 2023-01-12 15:21 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Victoria Dye, Jeff Hostetler
On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:
> As we'll see in a subsequent commit 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.
> - memset(&o->result, 0, sizeof(o->result));
> + index_state_init(&o->result);
> o->result.initialized = 1;
It's interesting that 'struct index_state' has an 'initialized'
member that we aren't setting in index_state_init(). Perhaps it's
only being used in special cases like this, and means something
more specific than "index_state_init() was run"? Or maybe we
could add it to INDEX_STATE_INIT and drop this line?
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()
2023-01-12 15:21 ` Derrick Stolee
@ 2023-01-12 16:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 16:47 ` Derrick Stolee
0 siblings, 1 reply; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 16:19 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git, Junio C Hamano, Victoria Dye, Jeff Hostetler
On Thu, Jan 12 2023, Derrick Stolee wrote:
> On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:
>> As we'll see in a subsequent commit 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.
>
>> - memset(&o->result, 0, sizeof(o->result));
>> + index_state_init(&o->result);
>> o->result.initialized = 1;
>
> It's interesting that 'struct index_state' has an 'initialized'
> member that we aren't setting in index_state_init(). Perhaps it's
> only being used in special cases like this, and means something
> more specific than "index_state_init() was run"? Or maybe we
> could add it to INDEX_STATE_INIT and drop this line?
It's unrelated, and doing that would be a bug. It's a bit unfortunately
named, a better name might be "read_index_data" or something.
It was added in 913e0e99b6a (unpack_trees(): protect the handcrafted
in-core index from read_cache(), 2008-08-23), which shows the use-case,
i.e. it's for avoiding re-reading the index file itself (or in that
case, to trust our hand-crafted faked-up version of it).
I opted not to mention it in the commit message, after being
sufficiently convinced that it was unrelated, which was probably a
mistake :)
Just as a sanity check, we do have really good test coverage of the
difference, at least 1/2 of the tests I bothered to wait for failed when
I tried this on top:
diff --git a/cache.h b/cache.h
index 4bf14e0bd94..1f8e5f4e823 100644
--- a/cache.h
+++ b/cache.h
@@ -371,6 +371,7 @@ struct index_state {
* "r" argument to index_state_init() in that case.
*/
#define INDEX_STATE_INIT(r) { \
+ .initialized = 1, \
.repo = (r), \
}
void index_state_init(struct index_state *istate, struct repository *r);
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()
2023-01-12 16:19 ` Ævar Arnfjörð Bjarmason
@ 2023-01-12 16:47 ` Derrick Stolee
0 siblings, 0 replies; 32+ messages in thread
From: Derrick Stolee @ 2023-01-12 16:47 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Victoria Dye, Jeff Hostetler
On 1/12/2023 11:19 AM, Ævar Arnfjörð Bjarmason wrote:
>
> On Thu, Jan 12 2023, Derrick Stolee wrote:
>> It's interesting that 'struct index_state' has an 'initialized'
>> member that we aren't setting in index_state_init(). Perhaps it's
>> only being used in special cases like this, and means something
>> more specific than "index_state_init() was run"? Or maybe we
>> could add it to INDEX_STATE_INIT and drop this line?
>
> It's unrelated, and doing that would be a bug. It's a bit unfortunately
> named, a better name might be "read_index_data" or something.
>
> It was added in 913e0e99b6a (unpack_trees(): protect the handcrafted
> in-core index from read_cache(), 2008-08-23), which shows the use-case,
> i.e. it's for avoiding re-reading the index file itself (or in that
> case, to trust our hand-crafted faked-up version of it).
>
> I opted not to mention it in the commit message, after being
> sufficiently convinced that it was unrelated, which was probably a
> mistake :)
>
> Just as a sanity check, we do have really good test coverage of the
> difference, at least 1/2 of the tests I bothered to wait for failed when
> I tried this on top:
>
> diff --git a/cache.h b/cache.h
> index 4bf14e0bd94..1f8e5f4e823 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -371,6 +371,7 @@ struct index_state {
> * "r" argument to index_state_init() in that case.
> */
> #define INDEX_STATE_INIT(r) { \
> + .initialized = 1, \
> .repo = (r), \
Thanks for the extra info. The patch is clearly correct with that
information.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 6/6] treewide: always have a valid "index_state.repo" member
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2023-01-12 12:55 ` [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index() Ævar Arnfjörð Bjarmason
@ 2023-01-12 12:55 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:22 ` Derrick Stolee
2023-01-12 15:23 ` [PATCH v2 0/6] cache API: always have a "istate->repo" Derrick Stolee
2023-01-13 19:22 ` Junio C Hamano
7 siblings, 1 reply; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 12:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
When the "repo" member was added to "the_index" in [1] the
repo_read_index() was made to populate it, but the unpopulated
"the_index" variable didn't get the same treatment.
Let's do that in initialize_the_repository() when we set it up, and
likewise for all of the current callers initialized an empty "struct
index_state".
This simplifies code that needs to deal with "the_index" or a custom
"struct index_state", we no longer need to second-guess this part of
the "index_state" deep in the stack. A recent example of such
second-guessing is the "istate->repo ? istate->repo : the_repository"
code in [2]. We can now simply use "istate->repo".
We're doing this by making use of the INDEX_STATE_INIT() macro (and
corresponding function) added in the preceding commit, which now have
mandatory "repo" arguments. As seen here there are cases where we set
it to NULL, but only if we're about to fill in the correct non-NULL
"repo" right afterwards.
For "fsmonitor-settings.c" we can remove the initialization of a NULL
"r" argument to "the_repository". This was added back in [3], and was
needed at the time for callers that would pass us the "r" from an
"istate->repo". Before this change such a change to
"fsmonitor-settings.c" would segfault all over the test suite (e.g. in
t0002-gitfile.sh).
This change has wider eventual implications for
"fsmonitor-settings.c". The reason the other lazy loading behavior in
it is required (starting with "if (!r->settings.fsmonitor) ..." is
because of the previously passed "r" being "NULL".
I have other local changes on top of this which move its configuration
reading to "prepare_repo_settings()" in "repo-settings.c", as we could
now start to rely on it being called for our "r". But let's leave all
of that for now, and narrowly remove this particular part of the
lazy-loading.
1. 1fd9ae517c4 (repository: add repo reference to index_state,
2021-01-23)
2. ee1f0c242ef (read-cache: add index.skipHash config option,
2023-01-06)
3. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,
2022-03-25)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
apply.c | 2 +-
builtin/difftool.c | 2 +-
builtin/sparse-checkout.c | 2 +-
builtin/stash.c | 8 ++++----
builtin/worktree.c | 2 +-
cache.h | 9 ++++++---
fsmonitor-settings.c | 14 --------------
fsmonitor.c | 2 +-
merge-recursive.c | 2 +-
read-cache.c | 17 +++++------------
repository.c | 4 +++-
revision.c | 2 +-
sparse-index.c | 9 ---------
split-index.c | 2 +-
unpack-trees.c | 2 +-
15 files changed, 27 insertions(+), 52 deletions(-)
diff --git a/apply.c b/apply.c
index 821fe411ec8..5eec433583e 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 = INDEX_STATE_INIT;
+ struct index_state result = INDEX_STATE_INIT(state->repo);
struct lock_file lock = LOCK_INIT;
int res;
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 758e7bd3b6a..dbbfb19f192 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 = INDEX_STATE_INIT;
+ struct index_state wtindex = INDEX_STATE_INIT(the_repository);
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 dc332c6d05f..c3738154918 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -217,7 +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);
+ index_state_init(&o.result, r);
o.skip_sparse_checkout = 0;
o.pl = pl;
diff --git a/builtin/stash.c b/builtin/stash.c
index a5f13fb1e9f..839569a9803 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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
cp_upd_index.git_cmd = 1;
strvec_pushl(&cp_upd_index.args, "update-index", "-z", "--add",
@@ -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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
if (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,
0, NULL)) {
@@ -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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
char *old_index_env = NULL, *old_repo_index_file;
remove_path(stash_index_path.buf);
@@ -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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
init_revisions(&rev, NULL);
copy_pathspec(&rev.prune_data, ps);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 311d6e90750..f51c40f1e1e 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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
struct strbuf path = STRBUF_INIT;
int i, found_submodules = 0;
diff --git a/cache.h b/cache.h
index be28fce12ac..4bf14e0bd94 100644
--- a/cache.h
+++ b/cache.h
@@ -367,10 +367,13 @@ struct index_state {
* 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.
+ * the end. The discard_index() will use its own "istate->repo" as the
+ * "r" argument to index_state_init() in that case.
*/
-#define INDEX_STATE_INIT { 0 }
-void index_state_init(struct index_state *istate);
+#define INDEX_STATE_INIT(r) { \
+ .repo = (r), \
+}
+void index_state_init(struct index_state *istate, struct repository *r);
void release_index(struct index_state *istate);
/* Name hashing */
diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c
index ee63a97dc51..899bfe9c813 100644
--- a/fsmonitor-settings.c
+++ b/fsmonitor-settings.c
@@ -143,8 +143,6 @@ static void lookup_fsmonitor_settings(struct repository *r)
enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
lookup_fsmonitor_settings(r);
@@ -153,8 +151,6 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
const char *fsm_settings__get_hook_path(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
lookup_fsmonitor_settings(r);
@@ -174,8 +170,6 @@ void fsm_settings__set_ipc(struct repository *r)
* Caller requested IPC explicitly, so avoid (possibly
* recursive) config lookup.
*/
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -197,8 +191,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)
* Caller requested hook explicitly, so avoid (possibly
* recursive) config lookup.
*/
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -210,8 +202,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)
void fsm_settings__set_disabled(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -223,8 +213,6 @@ void fsm_settings__set_disabled(struct repository *r)
void fsm_settings__set_incompatible(struct repository *r,
enum fsmonitor_reason reason)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -235,8 +223,6 @@ void fsm_settings__set_incompatible(struct repository *r,
enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
lookup_fsmonitor_settings(r);
diff --git a/fsmonitor.c b/fsmonitor.c
index 08af00c7387..a5b9e75437b 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -304,7 +304,7 @@ void refresh_fsmonitor(struct index_state *istate)
char *buf;
unsigned int i;
int is_trivial = 0;
- struct repository *r = istate->repo ? istate->repo : the_repository;
+ struct repository *r = istate->repo;
enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
enum fsmonitor_reason reason = fsm_settings__get_reason(r);
diff --git a/merge-recursive.c b/merge-recursive.c
index 0948b3ea961..ae469f8cc81 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 = INDEX_STATE_INIT;
+ struct index_state tmp_index = INDEX_STATE_INIT(opt->repo);
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 d191741f600..7bd12afb38c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2300,8 +2300,6 @@ 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)
- return;
prepare_repo_settings(istate->repo);
if (!istate->repo->settings.command_requires_full_index &&
is_sparse_index_allowed(istate, 0))
@@ -2330,8 +2328,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
fd = open(path, O_RDONLY);
if (fd < 0) {
if (!must_exist && errno == ENOENT) {
- if (!istate->repo)
- istate->repo = the_repository;
set_new_index_sparsity(istate);
return 0;
}
@@ -2433,9 +2429,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
trace2_data_intmax("index", the_repository, "read/cache_nr",
istate->cache_nr);
- if (!istate->repo)
- istate->repo = the_repository;
-
/*
* If the command explicitly requires a full index, force it
* to be full. Otherwise, correct the sparsity based on repository
@@ -2501,7 +2494,7 @@ int read_index_from(struct index_state *istate, const char *path,
release_index(split_index->base);
else
ALLOC_ARRAY(split_index->base, 1);
- index_state_init(split_index->base);
+ index_state_init(split_index->base, istate->repo);
base_oid_hex = oid_to_hex(&split_index->base_oid);
base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex);
@@ -2540,9 +2533,9 @@ int is_index_unborn(struct index_state *istate)
return (!istate->cache_nr && !istate->timestamp.sec);
}
-void index_state_init(struct index_state *istate)
+void index_state_init(struct index_state *istate, struct repository *r)
{
- struct index_state blank = INDEX_STATE_INIT;
+ struct index_state blank = INDEX_STATE_INIT(r);
memcpy(istate, &blank, sizeof(*istate));
}
@@ -2579,7 +2572,7 @@ void release_index(struct index_state *istate)
void discard_index(struct index_state *istate)
{
release_index(istate);
- index_state_init(istate);
+ index_state_init(istate, istate->repo);
}
/*
@@ -2933,7 +2926,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
int ieot_entries = 1;
struct index_entry_offset_table *ieot = NULL;
int nr, nr_threads;
- struct repository *r = istate->repo ? istate->repo : the_repository;
+ struct repository *r = istate->repo;
f = hashfd(tempfile->fd, tempfile->filename.buf);
diff --git a/repository.c b/repository.c
index a5805fa33b1..0e15220e6ac 100644
--- a/repository.c
+++ b/repository.c
@@ -28,6 +28,8 @@ void initialize_the_repository(void)
the_repo.remote_state = remote_state_new();
the_repo.parsed_objects = parsed_object_pool_new();
+ index_state_init(&the_index, the_repository);
+
repo_set_hash_algo(&the_repo, GIT_HASH_SHA1);
}
@@ -304,7 +306,7 @@ int repo_read_index(struct repository *repo)
if (!repo->index) {
ALLOC_ARRAY(repo->index, 1);
- index_state_init(repo->index);
+ index_state_init(repo->index, NULL /* set below */);
}
/* Complete the double-reference */
diff --git a/revision.c b/revision.c
index fb090886f5b..21f5f572c22 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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(revs->repo);
if (wt->is_current)
continue; /* current index already taken care of */
diff --git a/sparse-index.c b/sparse-index.c
index 86e3b99870b..147a13386a4 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -128,9 +128,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)
if (!core_apply_sparse_checkout || !core_sparse_checkout_cone)
return 0;
- if (!istate->repo)
- istate->repo = the_repository;
-
if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
int test_env;
@@ -327,9 +324,6 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)
pl = NULL;
}
- if (!istate->repo)
- istate->repo = the_repository;
-
/*
* A NULL pattern set indicates we are expanding a full index, so
* we use a special region name that indicates the full expansion.
@@ -552,9 +546,6 @@ void expand_to_path(struct index_state *istate,
if (!istate->sparse_index)
return;
- if (!istate->repo)
- istate->repo = the_repository;
-
in_expand_to_path = 1;
/*
diff --git a/split-index.c b/split-index.c
index a5b56c0c37f..5d0f04763ea 100644
--- a/split-index.c
+++ b/split-index.c
@@ -91,7 +91,7 @@ void move_cache_to_base_index(struct index_state *istate)
}
ALLOC_ARRAY(si->base, 1);
- index_state_init(si->base);
+ index_state_init(si->base, istate->repo);
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 d1d1b0f319e..3d05e45a279 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);
}
- index_state_init(&o->result);
+ index_state_init(&o->result, o->src_index->repo);
o->result.initialized = 1;
o->result.timestamp.sec = o->src_index->timestamp.sec;
o->result.timestamp.nsec = o->src_index->timestamp.nsec;
--
2.39.0.1205.g2ca064edc27
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 6/6] treewide: always have a valid "index_state.repo" member
2023-01-12 12:55 ` [PATCH v2 6/6] treewide: always have a valid "index_state.repo" member Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:22 ` Derrick Stolee
0 siblings, 0 replies; 32+ messages in thread
From: Derrick Stolee @ 2023-01-12 15:22 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Victoria Dye, Jeff Hostetler
On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:
> We're doing this by making use of the INDEX_STATE_INIT() macro (and
> corresponding function) added in the preceding commit, which now have
> mandatory "repo" arguments. As seen here there are cases where we set
> it to NULL, but only if we're about to fill in the correct non-NULL
> "repo" right afterwards.
The changes in part 5 to depend on INDEX_STATE_INIT and
index_state_init() make the changes in this patch much more
mechanical and easy to follow.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/6] cache API: always have a "istate->repo"
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2023-01-12 12:55 ` [PATCH v2 6/6] treewide: always have a valid "index_state.repo" member Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:23 ` Derrick Stolee
2023-01-13 18:29 ` Junio C Hamano
2023-01-13 19:22 ` Junio C Hamano
7 siblings, 1 reply; 32+ messages in thread
From: Derrick Stolee @ 2023-01-12 15:23 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Victoria Dye, Jeff Hostetler
On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote:
> * Derrick suggested in
> https://lore.kernel.org/git/6b92fad2-6b74-fddb-679c-8c8735e7103d@github.com/
> that things might be nicer if we had an explicit initializer, which
> was also the subject of the previous discussion at
> https://lore.kernel.org/git/xmqqmt6vqo2w.fsf@gitster.g/; but
> concluded that it was probably best to leave it for now.
>
> I tried it out, and I think it's worth just doing that now, which
> is why there's a new 5/6 here: We start by adding an
> INDEX_STATE_INIT macro, and corresponding function.
>
> There's a bit of churn in 6/6 as all of those now will take a
> "repo" argument, but I think the end result is worth it, because
> even if "repo" remains the only thing that we need to initialize
> we're now able to use ALLOC_ARRAY() instead of CALLOC_ARRAY().
>
> We'll thus be helped by analysis tools (which would show access to
> un-init'd memory) if we miss properly init-ing not just the "repo"
> field, but anything in the structure, so our test coverage will be
> better.
>
> It also makes the code easier to follow and change, as it's now
> more obvious where we initialize this, and it'll be easier to
> change it in the future if e.g. we add a new member that has
> mandatory initialization (e.g. a "struct strbuf").
I wasn't expecting the initializer idea to work as well as it has.
Now, Patch 5 does all the heavy lifting and Patch 6 is an easy read.
Outside of one question about the istate->initialized member (which
might not need any change) I'm happy with this version.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 0/6] cache API: always have a "istate->repo"
2023-01-12 15:23 ` [PATCH v2 0/6] cache API: always have a "istate->repo" Derrick Stolee
@ 2023-01-13 18:29 ` Junio C Hamano
0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2023-01-13 18:29 UTC (permalink / raw)
To: Derrick Stolee
Cc: Ævar Arnfjörð Bjarmason, git, Victoria Dye,
Jeff Hostetler
Derrick Stolee <derrickstolee@github.com> writes:
> I wasn't expecting the initializer idea to work as well as it has.
> Now, Patch 5 does all the heavy lifting and Patch 6 is an easy read.
>
> Outside of one question about the istate->initialized member (which
> might not need any change) I'm happy with this version.
Thanks, both. And especially for finding 913e0e99 (unpack_trees():
protect the handcrafted in-core index from read_cache(),
2008-08-23).
Will queue.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/6] cache API: always have a "istate->repo"
2023-01-12 12:55 ` [PATCH v2 0/6] " Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2023-01-12 15:23 ` [PATCH v2 0/6] cache API: always have a "istate->repo" Derrick Stolee
@ 2023-01-13 19:22 ` Junio C Hamano
2023-01-16 13:38 ` Ævar Arnfjörð Bjarmason
7 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2023-01-13 19:22 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Derrick Stolee, Victoria Dye, Jeff Hostetler
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> The "struct index_state" contains a "repo" member, which should be a
> pointer to the repository for that index, but which due to us
> constructing such structs on an ad-hoc basis in various places wasn't
> always available.
I'd exclude 6/6 for now, as it seems to depend on some changes only
in 'next'. Feel free to resend only that step with reduced scope so
that it applies to 'master', and send in incremental updates when
each of these topics that are only in 'next' graduates.
Thanks.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 0/6] cache API: always have a "istate->repo"
2023-01-13 19:22 ` Junio C Hamano
@ 2023-01-16 13:38 ` Ævar Arnfjörð Bjarmason
2023-01-16 16:53 ` Philip Oakley
0 siblings, 1 reply; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-16 13:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Derrick Stolee, Victoria Dye, Jeff Hostetler
On Fri, Jan 13 2023, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> The "struct index_state" contains a "repo" member, which should be a
>> pointer to the repository for that index, but which due to us
>> constructing such structs on an ad-hoc basis in various places wasn't
>> always available.
>
> I'd exclude 6/6 for now, as it seems to depend on some changes only
> in 'next'. Feel free to resend only that step with reduced scope so
> that it applies to 'master', and send in incremental updates when
> each of these topics that are only in 'next' graduates.
Okey, the 6/6 requires ds/omit-trailing-hash-in-index. As both it and
the 1-5/6 of this are in "next" now I think it's best that I just submit
the 6/6 stand-alone after both of those have graduated.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/6] cache API: always have a "istate->repo"
2023-01-16 13:38 ` Ævar Arnfjörð Bjarmason
@ 2023-01-16 16:53 ` Philip Oakley
2023-01-16 18:39 ` Junio C Hamano
0 siblings, 1 reply; 32+ messages in thread
From: Philip Oakley @ 2023-01-16 16:53 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, Junio C Hamano
Cc: git, Derrick Stolee, Victoria Dye, Jeff Hostetler
On 16/01/2023 13:38, Ævar Arnfjörð Bjarmason wrote:
> On Fri, Jan 13 2023, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> The "struct index_state" contains a "repo" member, which should be a
>>> pointer to the repository for that index, but which due to us
>>> constructing such structs on an ad-hoc basis in various places wasn't
>>> always available.
>> I'd exclude 6/6 for now, as it seems to depend on some changes only
>> in 'next'. Feel free to resend only that step with reduced scope so
>> that it applies to 'master', and send in incremental updates when
>> each of these topics that are only in 'next' graduates.
> Okey, the 6/6 requires ds/omit-trailing-hash-in-index. As both it and
> the 1-5/6 of this are in "next" now I think it's best that I just submit
> the 6/6 stand-alone after both of those have graduated.
>
micro-nit: The commit message of 5/6 starts "As we'll see in a
subsequent commit..", which may need a slight tweak if 6/6 becomes 'far
away' in the commit tree.
--
Philip
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/6] cache API: always have a "istate->repo"
2023-01-16 16:53 ` Philip Oakley
@ 2023-01-16 18:39 ` Junio C Hamano
2023-01-17 13:57 ` [PATCH] treewide: always have a valid "index_state.repo" member Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2023-01-16 18:39 UTC (permalink / raw)
To: Philip Oakley
Cc: Ævar Arnfjörð Bjarmason, git, Derrick Stolee,
Victoria Dye, Jeff Hostetler
Philip Oakley <philipoakley@iee.email> writes:
> On 16/01/2023 13:38, Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Jan 13 2023, Junio C Hamano wrote:
>>
>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>>
>>>> The "struct index_state" contains a "repo" member, which should be a
>>>> pointer to the repository for that index, but which due to us
>>>> constructing such structs on an ad-hoc basis in various places wasn't
>>>> always available.
>>> I'd exclude 6/6 for now, as it seems to depend on some changes only
>>> in 'next'. Feel free to resend only that step with reduced scope so
>>> that it applies to 'master', and send in incremental updates when
>>> each of these topics that are only in 'next' graduates.
>> Okey, the 6/6 requires ds/omit-trailing-hash-in-index. As both it and
>> the 1-5/6 of this are in "next" now I think it's best that I just submit
>> the 6/6 stand-alone after both of those have graduated.
>>
> micro-nit: The commit message of 5/6 starts "As we'll see in a
> subsequent commit..", which may need a slight tweak if 6/6 becomes 'far
> away' in the commit tree.
Indeed.
I'd use
Hopefully in some not so distant future, we'll get
advantages from always initializing the "repo" member ...
for now.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH] treewide: always have a valid "index_state.repo" member
2023-01-16 18:39 ` Junio C Hamano
@ 2023-01-17 13:57 ` Ævar Arnfjörð Bjarmason
2023-01-17 15:34 ` Derrick Stolee
0 siblings, 1 reply; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-17 13:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Victoria Dye, Jeff Hostetler,
Ævar Arnfjörð Bjarmason
When the "repo" member was added to "the_index" in [1] the
repo_read_index() was made to populate it, but the unpopulated
"the_index" variable didn't get the same treatment.
Let's do that in initialize_the_repository() when we set it up, and
likewise for all of the current callers initialized an empty "struct
index_state".
This simplifies code that needs to deal with "the_index" or a custom
"struct index_state", we no longer need to second-guess this part of
the "index_state" deep in the stack. A recent example of such
second-guessing is the "istate->repo ? istate->repo : the_repository"
code in [2]. We can now simply use "istate->repo".
We're doing this by making use of the INDEX_STATE_INIT() macro (and
corresponding function) added in [3], which now have mandatory "repo"
arguments.
Because we now call index_state_init() in repository.c's
initialize_the_repository() we don't need to handle the case where we
have a "repo->index" whose "repo" member doesn't match the "repo"
we're setting up, i.e. the "Complete the double-reference" code in
repo_read_index() being altered here. That logic was originally added
in [1], and was working around the lack of what we now have in
initialize_the_repository().
For "fsmonitor-settings.c" we can remove the initialization of a NULL
"r" argument to "the_repository". This was added back in [4], and was
needed at the time for callers that would pass us the "r" from an
"istate->repo". Before this change such a change to
"fsmonitor-settings.c" would segfault all over the test suite (e.g. in
t0002-gitfile.sh).
This change has wider eventual implications for
"fsmonitor-settings.c". The reason the other lazy loading behavior in
it is required (starting with "if (!r->settings.fsmonitor) ..." is
because of the previously passed "r" being "NULL".
I have other local changes on top of this which move its configuration
reading to "prepare_repo_settings()" in "repo-settings.c", as we could
now start to rely on it being called for our "r". But let's leave all
of that for now, and narrowly remove this particular part of the
lazy-loading.
1. 1fd9ae517c4 (repository: add repo reference to index_state,
2021-01-23)
2. ee1f0c242ef (read-cache: add index.skipHash config option,
2023-01-06)
3. 2f6b1eb794e (cache API: add a "INDEX_STATE_INIT" macro/function,
add release_index(), 2023-01-12)
4. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,
2022-03-25)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
This an updated verison of the 6/6 of [A], which per Junio's [B]
wasn't picked up with those patches, which are now in "next".
Junio: Now that Derrick's ds/omit-trailing-hash-in-index has landed on
"master" this can be applied on top a merge of "master" and what you
have in "ab/cache-api-cleanup" (that topic itself being based on a
too-old "master").
Since the v2 I changed the "Complete the double-reference" logic in
repo_read_index() so that we're not working around a state of a
affairs that no longer exists with this change.
A. https://lore.kernel.org/git/cover-v2-0.6-00000000000-20230112T124842Z-avarab@gmail.com/
B. https://lore.kernel.org/git/xmqqtu0u2q9u.fsf@gitster.g/
Range-diff:
1: 291233b0420 ! 1: b4998652822 treewide: always have a valid "index_state.repo" member
@@ Commit message
code in [2]. We can now simply use "istate->repo".
We're doing this by making use of the INDEX_STATE_INIT() macro (and
- corresponding function) added in the preceding commit, which now have
- mandatory "repo" arguments. As seen here there are cases where we set
- it to NULL, but only if we're about to fill in the correct non-NULL
- "repo" right afterwards.
+ corresponding function) added in [3], which now have mandatory "repo"
+ arguments.
+
+ Because we now call index_state_init() in repository.c's
+ initialize_the_repository() we don't need to handle the case where we
+ have a "repo->index" whose "repo" member doesn't match the "repo"
+ we're setting up, i.e. the "Complete the double-reference" code in
+ repo_read_index() being altered here. That logic was originally added
+ in [1], and was working around the lack of what we now have in
+ initialize_the_repository().
For "fsmonitor-settings.c" we can remove the initialization of a NULL
- "r" argument to "the_repository". This was added back in [3], and was
+ "r" argument to "the_repository". This was added back in [4], and was
needed at the time for callers that would pass us the "r" from an
"istate->repo". Before this change such a change to
"fsmonitor-settings.c" would segfault all over the test suite (e.g. in
@@ Commit message
2021-01-23)
2. ee1f0c242ef (read-cache: add index.skipHash config option,
2023-01-06)
- 3. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,
+ 3. 2f6b1eb794e (cache API: add a "INDEX_STATE_INIT" macro/function,
+ add release_index(), 2023-01-12)
+ 4. 1e0ea5c4316 (fsmonitor: config settings are repository-specific,
2022-03-25)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ repository.c: void initialize_the_repository(void)
}
@@ repository.c: int repo_read_index(struct repository *repo)
+ {
+ int res;
++ /* Complete the double-reference */
if (!repo->index) {
ALLOC_ARRAY(repo->index, 1);
- index_state_init(repo->index);
-+ index_state_init(repo->index, NULL /* set below */);
- }
+- }
+-
+- /* Complete the double-reference */
+- if (!repo->index->repo)
+- repo->index->repo = repo;
+- else if (repo->index->repo != repo)
++ index_state_init(repo->index, repo);
++ } else if (repo->index->repo != repo) {
+ BUG("repo's index should point back at itself");
++ }
+
+ res = read_index_from(repo->index, repo->index_file, repo->gitdir);
- /* Complete the double-reference */
## revision.c ##
@@ revision.c: void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
apply.c | 2 +-
builtin/difftool.c | 2 +-
builtin/sparse-checkout.c | 2 +-
builtin/stash.c | 8 ++++----
builtin/worktree.c | 2 +-
cache.h | 9 ++++++---
fsmonitor-settings.c | 14 --------------
fsmonitor.c | 2 +-
merge-recursive.c | 2 +-
read-cache.c | 17 +++++------------
repository.c | 13 ++++++-------
revision.c | 2 +-
sparse-index.c | 9 ---------
split-index.c | 2 +-
unpack-trees.c | 2 +-
15 files changed, 30 insertions(+), 58 deletions(-)
diff --git a/apply.c b/apply.c
index 821fe411ec8..5eec433583e 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 = INDEX_STATE_INIT;
+ struct index_state result = INDEX_STATE_INIT(state->repo);
struct lock_file lock = LOCK_INIT;
int res;
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 758e7bd3b6a..dbbfb19f192 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 = INDEX_STATE_INIT;
+ struct index_state wtindex = INDEX_STATE_INIT(the_repository);
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 dc332c6d05f..c3738154918 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -217,7 +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);
+ index_state_init(&o.result, r);
o.skip_sparse_checkout = 0;
o.pl = pl;
diff --git a/builtin/stash.c b/builtin/stash.c
index a5f13fb1e9f..839569a9803 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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
cp_upd_index.git_cmd = 1;
strvec_pushl(&cp_upd_index.args, "update-index", "-z", "--add",
@@ -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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
if (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,
0, NULL)) {
@@ -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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
char *old_index_env = NULL, *old_repo_index_file;
remove_path(stash_index_path.buf);
@@ -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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
init_revisions(&rev, NULL);
copy_pathspec(&rev.prune_data, ps);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 311d6e90750..f51c40f1e1e 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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(the_repository);
struct strbuf path = STRBUF_INIT;
int i, found_submodules = 0;
diff --git a/cache.h b/cache.h
index be28fce12ac..4bf14e0bd94 100644
--- a/cache.h
+++ b/cache.h
@@ -367,10 +367,13 @@ struct index_state {
* 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.
+ * the end. The discard_index() will use its own "istate->repo" as the
+ * "r" argument to index_state_init() in that case.
*/
-#define INDEX_STATE_INIT { 0 }
-void index_state_init(struct index_state *istate);
+#define INDEX_STATE_INIT(r) { \
+ .repo = (r), \
+}
+void index_state_init(struct index_state *istate, struct repository *r);
void release_index(struct index_state *istate);
/* Name hashing */
diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c
index ee63a97dc51..899bfe9c813 100644
--- a/fsmonitor-settings.c
+++ b/fsmonitor-settings.c
@@ -143,8 +143,6 @@ static void lookup_fsmonitor_settings(struct repository *r)
enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
lookup_fsmonitor_settings(r);
@@ -153,8 +151,6 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
const char *fsm_settings__get_hook_path(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
lookup_fsmonitor_settings(r);
@@ -174,8 +170,6 @@ void fsm_settings__set_ipc(struct repository *r)
* Caller requested IPC explicitly, so avoid (possibly
* recursive) config lookup.
*/
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -197,8 +191,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)
* Caller requested hook explicitly, so avoid (possibly
* recursive) config lookup.
*/
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -210,8 +202,6 @@ void fsm_settings__set_hook(struct repository *r, const char *path)
void fsm_settings__set_disabled(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -223,8 +213,6 @@ void fsm_settings__set_disabled(struct repository *r)
void fsm_settings__set_incompatible(struct repository *r,
enum fsmonitor_reason reason)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
r->settings.fsmonitor = alloc_settings();
@@ -235,8 +223,6 @@ void fsm_settings__set_incompatible(struct repository *r,
enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)
{
- if (!r)
- r = the_repository;
if (!r->settings.fsmonitor)
lookup_fsmonitor_settings(r);
diff --git a/fsmonitor.c b/fsmonitor.c
index 08af00c7387..a5b9e75437b 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -304,7 +304,7 @@ void refresh_fsmonitor(struct index_state *istate)
char *buf;
unsigned int i;
int is_trivial = 0;
- struct repository *r = istate->repo ? istate->repo : the_repository;
+ struct repository *r = istate->repo;
enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
enum fsmonitor_reason reason = fsm_settings__get_reason(r);
diff --git a/merge-recursive.c b/merge-recursive.c
index 0948b3ea961..ae469f8cc81 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 = INDEX_STATE_INIT;
+ struct index_state tmp_index = INDEX_STATE_INIT(opt->repo);
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 d191741f600..7bd12afb38c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2300,8 +2300,6 @@ 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)
- return;
prepare_repo_settings(istate->repo);
if (!istate->repo->settings.command_requires_full_index &&
is_sparse_index_allowed(istate, 0))
@@ -2330,8 +2328,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
fd = open(path, O_RDONLY);
if (fd < 0) {
if (!must_exist && errno == ENOENT) {
- if (!istate->repo)
- istate->repo = the_repository;
set_new_index_sparsity(istate);
return 0;
}
@@ -2433,9 +2429,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
trace2_data_intmax("index", the_repository, "read/cache_nr",
istate->cache_nr);
- if (!istate->repo)
- istate->repo = the_repository;
-
/*
* If the command explicitly requires a full index, force it
* to be full. Otherwise, correct the sparsity based on repository
@@ -2501,7 +2494,7 @@ int read_index_from(struct index_state *istate, const char *path,
release_index(split_index->base);
else
ALLOC_ARRAY(split_index->base, 1);
- index_state_init(split_index->base);
+ index_state_init(split_index->base, istate->repo);
base_oid_hex = oid_to_hex(&split_index->base_oid);
base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex);
@@ -2540,9 +2533,9 @@ int is_index_unborn(struct index_state *istate)
return (!istate->cache_nr && !istate->timestamp.sec);
}
-void index_state_init(struct index_state *istate)
+void index_state_init(struct index_state *istate, struct repository *r)
{
- struct index_state blank = INDEX_STATE_INIT;
+ struct index_state blank = INDEX_STATE_INIT(r);
memcpy(istate, &blank, sizeof(*istate));
}
@@ -2579,7 +2572,7 @@ void release_index(struct index_state *istate)
void discard_index(struct index_state *istate)
{
release_index(istate);
- index_state_init(istate);
+ index_state_init(istate, istate->repo);
}
/*
@@ -2933,7 +2926,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
int ieot_entries = 1;
struct index_entry_offset_table *ieot = NULL;
int nr, nr_threads;
- struct repository *r = istate->repo ? istate->repo : the_repository;
+ struct repository *r = istate->repo;
f = hashfd(tempfile->fd, tempfile->filename.buf);
diff --git a/repository.c b/repository.c
index a5805fa33b1..937fa974b38 100644
--- a/repository.c
+++ b/repository.c
@@ -28,6 +28,8 @@ void initialize_the_repository(void)
the_repo.remote_state = remote_state_new();
the_repo.parsed_objects = parsed_object_pool_new();
+ index_state_init(&the_index, the_repository);
+
repo_set_hash_algo(&the_repo, GIT_HASH_SHA1);
}
@@ -302,16 +304,13 @@ int repo_read_index(struct repository *repo)
{
int res;
+ /* Complete the double-reference */
if (!repo->index) {
ALLOC_ARRAY(repo->index, 1);
- index_state_init(repo->index);
- }
-
- /* Complete the double-reference */
- if (!repo->index->repo)
- repo->index->repo = repo;
- else if (repo->index->repo != repo)
+ index_state_init(repo->index, repo);
+ } else if (repo->index->repo != repo) {
BUG("repo's index should point back at itself");
+ }
res = read_index_from(repo->index, repo->index_file, repo->gitdir);
diff --git a/revision.c b/revision.c
index fb090886f5b..21f5f572c22 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 = INDEX_STATE_INIT;
+ struct index_state istate = INDEX_STATE_INIT(revs->repo);
if (wt->is_current)
continue; /* current index already taken care of */
diff --git a/sparse-index.c b/sparse-index.c
index 86e3b99870b..147a13386a4 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -128,9 +128,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)
if (!core_apply_sparse_checkout || !core_sparse_checkout_cone)
return 0;
- if (!istate->repo)
- istate->repo = the_repository;
-
if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
int test_env;
@@ -327,9 +324,6 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)
pl = NULL;
}
- if (!istate->repo)
- istate->repo = the_repository;
-
/*
* A NULL pattern set indicates we are expanding a full index, so
* we use a special region name that indicates the full expansion.
@@ -552,9 +546,6 @@ void expand_to_path(struct index_state *istate,
if (!istate->sparse_index)
return;
- if (!istate->repo)
- istate->repo = the_repository;
-
in_expand_to_path = 1;
/*
diff --git a/split-index.c b/split-index.c
index a5b56c0c37f..5d0f04763ea 100644
--- a/split-index.c
+++ b/split-index.c
@@ -91,7 +91,7 @@ void move_cache_to_base_index(struct index_state *istate)
}
ALLOC_ARRAY(si->base, 1);
- index_state_init(si->base);
+ index_state_init(si->base, istate->repo);
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 d1d1b0f319e..3d05e45a279 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);
}
- index_state_init(&o->result);
+ index_state_init(&o->result, o->src_index->repo);
o->result.initialized = 1;
o->result.timestamp.sec = o->src_index->timestamp.sec;
o->result.timestamp.nsec = o->src_index->timestamp.nsec;
--
2.39.0.1225.g30a3d88132d
^ permalink raw reply related [flat|nested] 32+ messages in thread