Re: [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()

[Derrick Stolee <derrickstolee@github.com>]

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