Re: [PATCH] read-cache: use index state repository for trace2 logging

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <stolee@gmail.com> writes:

> On 3/27/2026 6:08 AM, Jayesh Daga via GitGitGadget wrote:
>
>>     Robustness: The ternary fallback ensures we avoid potential NULL pointer
>>     dereferences while maintaining existing logging behavior in edge cases.
>
>> +	r = istate->repo ? istate->repo : the_repository;
>
> If I understand correctly, it is a bug if istate->repo is NULL.

Because INDEX_STATE_INIT(r) assigns the repository as the first
thing, I tend to agree.  A (bare) repository can lack the index
so repo->index might be NULL, but if you have an istate instance,
it should always know which repository it came from.

>> +	trace2_data_intmax("index", r, "read/version",
>>  			   istate->version);
>> -	trace2_data_intmax("index", the_repository, "read/cache_nr",
>> +	trace2_data_intmax("index", r, "read/cache_nr",
>>  			   istate->cache_nr);
>
> Other than that, this is a minor improvement in the right direction. I'd
> rather that it be more complete if you are working in this file.

;-)  Long timers always aim higher than posted patches.

> If you are already working in this space, then I recommend figuring out
> how much we can rely on istate->repo and then apply that knowledge to
> these cases as separate commits:
>
> 1. Replace the uses in the trace2 calls with istate->repo
>    and delete the TODO comments.
>
> 2. Replace the other uses of the_repository when an istate
>    exists already.

Excellent suggestion.
Thanks.

By the way, Jeyesh, do you really want to be known with a numbered
"jayesh0104" as your name?  These author identities are cast in
stone in commit objects and will stay with the project.

Also see Documentation/SubmittingPatches::[dco,real-name].