Re: [PATCH v3] packed-refs: use `fwrite()` when passing refs verbatim

> On Tue, Oct 06, 2026 at 05:11:57PM -0400, Karthik Nayak wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> > On Tue, Oct 06, 2026 at 11:18:40AM +0200, Karthik Nayak wrote:
>> >> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
>> >> index a73fc6aca7..43ad674cf4 100644
>> >> --- a/refs/packed-backend.c
>> >> +++ b/refs/packed-backend.c
>> >> @@ -879,6 +879,12 @@ struct packed_ref_iterator {
>> >>  	/* The current position in the snapshot's buffer: */
>> >>  	const char *pos;
>> >>
>> >> +	/*
>> >> +	 * Start of the current record, set when advancing `pos`. Used to
>> >> +	 * pass records verbatim to `fwrite()`.
>> >> +	 */
>> >> +	const char *record_start;
>> >
>> > The way this is written makes you think that `pos == record_start`, and
>> > thus one wonders why we even need this separate variable in the first
>> > place. So I assume that we modify `pos` in some cases without modifying
>> > the new variable at the same point in time. But if so, the above comment
>> > is not true anymore.
>> >
>>
>> Hmm. I only state that this is 'set _when_ advancing `pos`', Why do you
>> think that this would mean `pos == record_start`?
>
> To me it reads as "whenever we advance `pos`, then we set
> `record_start`". Which is not the case, we also sometimes advance `pos`
> without setting it.
>
> Patrick