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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2026, 16:28 UTC
Message-ID: <xmqqqzp5kzj3.fsf@gitster.g>
In-Reply-To: <770465fe-c38f-45a9-b1b0-0ad682a35fab@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 8 quoted lines

> 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.

Show 8 quoted lines

>> +	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.

Show 9 quoted lines

> 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].