Re: [PATCH v4] cache-tree.c: remove implicit dependency on the_repository
From: Junio C Hamano <hidden>
Date: 2021-04-07 23:03:08
"Chinmoy via GitGitGadget" [off-list ref] writes:
From: Chinmoy Chakraborty <redacted>
This kills the_repository dependency in cache_tree_update()
and prime_cache_tree().
Signed-off-by: Chinmoy Chakraborty <redacted>
---
Replace the_repository with rHuh???
quoted hunk ↗ jump to hunk
diff --git a/cache-tree.c b/cache-tree.c index add1f0771317..4928a9f0f13b 100644 --- a/cache-tree.c +++ b/cache-tree.c@@ -446,10 +446,10 @@ int cache_tree_update(struct index_state *istate, int flags) istate->cache_tree = cache_tree(); trace_performance_enter(); - trace2_region_enter("cache_tree", "update", the_repository); + trace2_region_enter("cache_tree", "update", istate->repo); i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, "", 0, &skip, flags); - trace2_region_leave("cache_tree", "update", the_repository); + trace2_region_leave("cache_tree", "update", istate->repo); trace_performance_leave("cache_tree_update"); if (i < 0) return i;@@ -746,13 +746,13 @@ void prime_cache_tree(struct repository *r, struct index_state *istate, struct tree *tree) { - trace2_region_enter("cache-tree", "prime_cache_tree", the_repository); + trace2_region_enter("cache-tree", "prime_cache_tree", r); cache_tree_free(&istate->cache_tree); istate->cache_tree = cache_tree(); prime_cache_tree_rec(r, istate->cache_tree, tree); istate->cache_changed |= CACHE_TREE_CHANGED; - trace2_region_leave("cache-tree", "prime_cache_tree", the_repository); + trace2_region_leave("cache-tree", "prime_cache_tree", r); }
The patch assumes that istate->repo will always set, but it does not even try to justify why that assumption is safe to make (e.g. "the entire codebase that leads to this function has been audited and made sure istate at this point will always have its .repo member is set" in the log message, if such an audit has actually been done, may have been convincing), which I find quite troubling.