From: Patrick Steinhardt Date: Wed, 07 Oct 2026 05:12:39 GMT Subject: Re: [PATCH v3] packed-refs: use `fwrite()` when passing refs verbatim Message-ID: In-Reply-To: On Tue, Oct 06, 2026 at 05:11:57PM -0400, Karthik Nayak wrote: > Patrick Steinhardt 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