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);
