Re: [PATCH v2 5/6] cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jan 12, 2023, 16:47 UTC
- Message-ID
- <ee711878-87d7-dc94-9913-681cbd6b67a5@github.com>
- In-Reply-To
- <230112.86zganzpwk.gmgdl@evledraar.gmail.com>
On 1/12/2023 11:19 AM, Ævar Arnfjörð Bjarmason wrote:
Show 34 quoted lines
>
> 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