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

[Patrick Steinhardt <ps@pks.im>]

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